Convert to IndexedDB#1663
Open
Travja wants to merge 16 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Migrates editor persistence from direct localStorage reads/writes to an IndexedDB-backed persistence layer (editor-persistence) with shared save/quota handling, plus adds Vitest-based tests to validate persistence behavior.
Changes:
- Added a new IndexedDB persistence module (migration from legacy
localStorage, read/write helpers, DB schema + IDB operations). - Updated skill/class/attribute stores and route loaders to hydrate from the new persistence layer and to centralize quota/“memory only” warning state.
- Added Vitest + jsdom configuration and new test suites for persistence and quota-state handling.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.ts | Configures Vitest to run with jsdom. |
| src/routes/+layout.ts | Ensures editor data is hydrated before the app runs. |
| src/routes/+layout.svelte | Adds persistence warnings/unsupported banner + formatting adjustments. |
| src/routes/(app)/[type=istype]/[id]/edit/+page.ts | Hydrates persistence before loading editor entities. |
| src/routes/(app)/[type=istype]/[id]/+page.ts | Hydrates persistence and loads skill via store API. |
| src/data/skill-store.svelte.ts | Switches skill persistence to editor-persistence + quota-state helpers. |
| src/data/persistence-state.ts | Introduces shared quota detection + save-state decision helpers. |
| src/data/persistence-state.test.ts | Tests quota detection and persistence save-state behavior. |
| src/data/editor-session.ts | Centralizes one-time hydration of skill/class/attribute stores. |
| src/data/editor-persistence.ts | Implements cache-backed IndexedDB persistence API + legacy migration flow. |
| src/data/editor-persistence.test.ts | Tests legacy migration and normalization for structured-clone safety. |
| src/data/editor-persistence-unsupported.test.ts | Tests behavior when IndexedDB is unavailable. |
| src/data/editor-persistence-shared.ts | Defines DB schema/constants and normalization helper. |
| src/data/editor-persistence-legacy.ts | Reads legacy localStorage formats and clears legacy storage. |
| src/data/editor-persistence-db.ts | Implements IndexedDB open/read/write/replace helpers using idb. |
| src/data/class-store.svelte.ts | Switches class persistence to editor-persistence + quota-state helpers. |
| src/data/attribute-store.ts | Switches attribute persistence to editor-persistence + hydration and quota flow. |
| package.json | Adds vitest, jsdom, fake-indexeddb, and idb plus test script. |
| package-lock.json | Locks new dependencies required for IndexedDB + testing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…view (#1664) Agent-Logs-Url: https://github.com/magemonkeystudio/fabled/sessions/02880ff6-5b6d-4382-9184-ec62a0ee62ed Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Travja <1574947+Travja@users.noreply.github.com>
Agent-Logs-Url: https://github.com/magemonkeystudio/fabled/sessions/22910d41-16e9-482f-ad1b-8ea3a9df324e Co-authored-by: Travja <1574947+Travja@users.noreply.github.com>
Agent-Logs-Url: https://github.com/magemonkeystudio/fabled/sessions/c37c678a-d10f-4976-b1ca-0afa47711871 Co-authored-by: Travja <1574947+Travja@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/magemonkeystudio/fabled/sessions/d1047098-d45a-4d3e-84d9-2b3ada02ce51 Co-authored-by: Travja <1574947+Travja@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/data/skill-store.svelte.ts:365
- This always logs
Saved <name>even when the IndexedDB write failed (including quota failures). Consider logging only onresult.ok, and/or logging a different message when persistence is skipped/failed so diagnostics match reality.
if (!result.ok) {
if (!result.quotaExceeded) {
console.error(this.name + ' Save error', result.error);
} else {
const persistState = finishPersistenceSave(
{
name: this.name,
tooBig: this.tooBig,
acknowledged: this.acknowledged
},
result
);
this.tooBig = persistState.state.tooBig;
this.acknowledged = persistState.state.acknowledged;
saveError.set(this);
}
} else {
const persistState = finishPersistenceSave(
{
name: this.name,
tooBig: this.tooBig,
acknowledged: this.acknowledged
},
result
);
this.previousName = this.name;
this.tooBig = persistState.state.tooBig;
this.acknowledged = persistState.state.acknowledged;
if (persistState.clearSaveError && get(saveError)?.name === this.name) {
saveError.set(undefined);
}
}
this.saveDebounceTimeout = undefined;
skillStore.isSaving.set(false);
console.log('Saved ' + this.name + ' 😎');
}, 600); // Adjust the debounce delay as needed
Removed the `tooBig` and `acknowledged` state from individual items (skills, classes, attributes) to streamline persistence error management. The UI now relies on a single `saveError` message for display. Improved cloning logic to ensure data is explicitly loaded before attempting a clone, preventing potential issues with incomplete data. Updated Prettier configuration to use spaces instead of tabs for consistent formatting.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a major refactor to the attribute and class data persistence logic, migrating from direct
localStorageusage to a new, more robust persistence layer. It also adds new dependencies and scripts to support testing and IndexedDB-based storage. The most significant changes are the replacement of direct storage access with new persistence service functions, improved error handling for storage quota issues, and code style improvements for consistency.Persistence Layer Refactor:
localStorageaccess inattribute-store.tsandclass-store.svelte.tswith new functions fromeditor-persistence(such assavePersistedAttributes,getPersistedAttribute,deletePersistedAttribute,savePersistedClass, etc.), centralizing and improving data storage logic. This enables better error handling, quota management, and future extensibility. [1] [2] [3] [4] [5]Code Quality and Consistency:
Testing and Dependency Updates:
idb,fake-indexeddb,jsdom, andvitestto support IndexedDB-backed storage and enable robust testing of the new persistence logic. [1] [2]testscript topackage.jsonfor running tests with Vitest.These changes lay the groundwork for more reliable, scalable, and testable data persistence in the application.