Migrate from localStorage to IndexedDB #639
Conversation
|
Preview build will be at |
Hopefully having this as a separate commit can make the diff easier to read.
Provide default values for project data and settings
Remove methods that are now unused.
Add new createdAt field used to sort actions and recordings when loaded from IndexedDB.
Can't use the migration function that creates uuids as the snapshot is different for every run.
microbit-robert
force-pushed
the
storage
branch
from
85ad1a2 to
b8fe124
Compare
Everything that updates multiple stores is now a transaction. Assert data loaded from storage. Update project 'updatedAt' when appropriate
src/storage.ts
Outdated
| const projectDataStore = tx.objectStore(DatabaseStore.PROJECT_DATA); | ||
| const projectData = assertData(await projectDataStore.get(this.projectId)); | ||
| const updatedActionIds = Array.from( | ||
| new Set(...actions.map((a) => a.id), projectData.actionIds) |
There was a problem hiding this comment.
Is this spread right?
> new Set(["1", "2"])
Set(2) { '1', '2' }
> new Set(...["1", "2"])
Set(1) { '1' }
src/storage.ts
Outdated
| return tx.done; | ||
| } | ||
|
|
||
| // TODO: TypeScript to ensure that a transaction with DatabaseStore.PROJECT_DATA is passed in. |
There was a problem hiding this comment.
I cannot find a TypeScript type that does the right thing here, so I've gone with just duplicating the code twice and removing this method 0809259
src/storage.ts
Outdated
| interface ProjectData { | ||
| id: string; | ||
| timestamp?: number; | ||
| actionIds: string[]; |
There was a problem hiding this comment.
Alternative modelling suggestion:
Potentially, data integrity would be better if we removed actionIds and recordingIds from records for project and action respectively, and instead had an projectId and actionId columns on action and recording, with a non-unique index for querying.
This would structurally enforce that actions/recordings aren't shared between projects and are always owned by a project. What do you think?
There was a problem hiding this comment.
So e.g. action querying would be something like:
const actionsForProject = await actionsStore.index('projectId').getAll(projectId);
There was a problem hiding this comment.
Yes, this seems worth trying.
There was a problem hiding this comment.
Changes made in b5b272f
Seems to work well, but would be better tested in the multiple projects branch. At the moment, each action and recording still have unique ids (we could go back to datetime, the one advantage of this would be that we wouldn't have to sort actions and recordings when pulling from IndexedDB)
Requires thorough testing and review. The current loading page is a simple proof-of-concept - this could probably be a bit nicer and handle the project load failure case as well.
The storage has error handling that isn't currently doing anything other than logging to the console.
TODO:
Date.nowto uuid for action and recording idsA more sensible diff discounting the white space shift from removing the old persist middleware, but still quite large.
This can now support multiple projects, and I'll create a new PR targeting this one to work as a proof of concept. This should now be a simple case of not clearing the stores for a new session and project import.
Turns out that the
upgradecallback should not be async and it's working by luck right now, so we need a new approach to migrate from localStorage / setup default values. Fixed in 3cdc10e