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.
Comment on lines
229
to
235
| deleteAttribute = (data: FabledAttribute) => { | ||
| const filtered = get(this.attributes).filter(c => c != data); | ||
| const filtered = get(this.attributes).filter((c) => c != data); | ||
| const act = get(active); | ||
| this.attributes.set(filtered); | ||
| this.saveAll(); | ||
| void deletePersistedAttribute(data.name); | ||
|
|
Comment on lines
+215
to
+233
| const db = await openEditorDatabase(); | ||
| const normalizedRecords = normalizeForPersistence(records); | ||
| await replaceIndexedDbData( | ||
| db, | ||
| { | ||
| skills: [...cache.skills.entries()].map(([name, data]) => ({ name, data })), | ||
| classes: [...cache.classes.entries()].map(([name, data]) => ({ name, data })), | ||
| attributes: normalizedRecords, | ||
| skillFolders: getPersistedFolders('skill'), | ||
| classFolders: getPersistedFolders('class') | ||
| }, | ||
| { | ||
| skills: [...cache.skills.keys()], | ||
| classes: [...cache.classes.keys()], | ||
| attributes: [...cache.attributes.keys()] | ||
| } | ||
| ); | ||
| replacePersistedAttributeCache(normalizedRecords); | ||
| return { ok: true, quotaExceeded: false }; |
Comment on lines
+196
to
204
| loadAttribute = async (data: FabledAttribute) => { | ||
| if (data.loaded) return; | ||
|
|
||
| if (data.location === 'local') { | ||
| const yamlData = <MultiAttributeYamlData>parseYaml(localStorage.getItem('attribs') || ''); | ||
| const yamlData = await getPersistedAttribute(data.name); | ||
| if (!yamlData) return; | ||
| const attrib = yamlData[data.name]; | ||
| data.load(attrib); | ||
| data.load(yamlData); | ||
| } | ||
| }; |
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>
Comment on lines
+216
to
+227
| try { | ||
| const db = await openEditorDatabase(); | ||
| const normalizedRecords = normalizeForPersistence(records); | ||
| await replaceIndexedDbData( | ||
| db, | ||
| { | ||
| skills: [...cache.skills.entries()].map(([name, data]) => ({ name, data })), | ||
| classes: [...cache.classes.entries()].map(([name, data]) => ({ name, data })), | ||
| attributes: normalizedRecords, | ||
| skillFolders: getPersistedFolders('skill'), | ||
| classFolders: getPersistedFolders('class') | ||
| }, |
Comment on lines
10
to
+19
| @@ -23,59 +16,5 @@ export const load: LayoutLoad = async ({ url }) => { | |||
| } | |||
| } | |||
|
|
|||
| if (url.host.includes('localhost')) return; | |||
|
|
|||
| if (url.searchParams.has('migrationData')) { | |||
| // Load the skills into the editor. | |||
| // This should be from migrations. | |||
|
|
|||
| getHaste({ url: url.searchParams.get('migrationData') || undefined }) | |||
| .then(data => { | |||
| const skillData = data.split(separator)[0]; | |||
| const classData = data.split(separator)[1]; | |||
| const skillFolders = data.split(separator)[2]; | |||
| const classFolders = data.split(separator)[3]; | |||
| const attributes = data.split(separator)[4]; | |||
|
|
|||
| parseYaml(skillData).forEach((skill: MultiSkillYamlData) => { | |||
| localStorage.setItem('sapi.skill.' + skill.name, YAML.stringify(skill, { | |||
| lineWidth: 0, | |||
| aliasDuplicateObjects: false | |||
| })); | |||
| }); | |||
| parseYaml(classData).forEach((cls: MultiSkillYamlData) => { | |||
| localStorage.setItem('sapi.class.' + cls.name, YAML.stringify(cls, { | |||
| lineWidth: 0, | |||
| aliasDuplicateObjects: false | |||
| })); | |||
| }); | |||
| localStorage.setItem('skillFolders', skillFolders); | |||
| localStorage.setItem('classFolders', classFolders); | |||
| localStorage.setItem('attribs', attributes); | |||
|
|
|||
| window.location.href = `https://${expectedHost}${base}`; | |||
| }) | |||
| .catch(console.error); | |||
|
|
|||
| return; | |||
| } | |||
|
|
|||
| // if (expectedHost.includes(url.host) || get(skills).length == 0) return; | |||
| // | |||
| // alert('We\'re migrating to a new URL. You\'re now going to be redirected. Your skills/classes should remain in tact.'); | |||
| // | |||
| // const skillYaml = YAML.stringify(await getAllSkillYaml(), { lineWidth: 0, aliasDuplicateObjects: false }); | |||
| // const classYaml = YAML.stringify(getAllClassYaml(), { lineWidth: 0, aliasDuplicateObjects: false }); | |||
| // const skillFolders = localStorage.getItem('skillFolders'); | |||
| // const classFolders = localStorage.getItem('classFolders'); | |||
| // const attributes = localStorage.getItem('attribs'); | |||
| // | |||
| // const qualifiedData = skillYaml + separator | |||
| // + classYaml + separator | |||
| // + skillFolders + separator | |||
| // + classFolders + separator | |||
| // + attributes; | |||
| // | |||
| // createPaste(qualifiedData) | |||
| // .then((url: string) => window.location.href = `https://${expectedHost}?migrationData=${url}`); | |||
| }; No newline at end of file | |||
| await hydrateEditorData(); | |||
| } from './editor-persistence-shared'; | ||
| import { | ||
| CLASS_FOLDERS_KEY, | ||
| SKILL_FOLDERS_KEY, |
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.