Repository navigation
feat(dartpad-preview): Introduce project session ids - #4006
dtetruash-nl wants to merge 5 commits into
Conversation
| Future<web.IDBDatabase>? _database; | ||
| bool _closed = false; | ||
|
|
||
| static const _nextSessionCounterKey = 'next'; |
There was a problem hiding this comment.
For Reviewer: Keyed the column to enable extending session store later. This just points to the session Id which would be given out next
| final request = web.window.indexedDB.open(databaseName, _currentDatabaseVersion); | ||
| request.onupgradeneeded = ((web.IDBVersionChangeEvent event) { | ||
| _migrateDatabase(request.result as web.IDBDatabase, fromVersion: event.oldVersion); |
There was a problem hiding this comment.
For Reviewer: Since the schema of indexedDB changed with the addition of the sessions table, catch the upgradeneeded event to follow through with database migration
There was a problem hiding this comment.
As we are still in preview, dont bother with migrations IMHO
| final freshId = store.entries.keys.singleWhere((id) => id != 'saved'); | ||
| await repositories.single.workspaceResourceApi.writeFileFromText('lib/main.dart', 'edited fresh project'); | ||
| await pumpEventQueue(); | ||
| await Future<void>.delayed(const Duration(milliseconds: 350)); |
There was a problem hiding this comment.
For Reviewer: Unrelated fix to have this test actually wait for the edit instead of hanging. Make it 350ms slower, so probably a better way to do this, but oh well for now
There was a problem hiding this comment.
this also fixes the failing CI test on #3991
| // Re-opening the same sourced URL without its `s` session id (e.g. a second | ||
| // tab, modeled here as a fresh App instance over the same store) must | ||
| // not reuse the first session id. | ||
| tester.pumpComponent(app('?sample=counter')); |
There was a problem hiding this comment.
For Reviewer: We need to test if a second app instance get a new session id. This is the simplest way I could think of doing so, any other way that you can see?
There was a problem hiding this comment.
For Reviewer: Went with storing the session Id counter in the DB over local storage to avoid races, keep the two persistence data in the same place, and avoid having to use a synchronous api
_projectUri was only read by the restoreProject closure removed in 96ffc77. Drop the field and the unused dartpad_editor import. Co-Authored-By: Claude Code <noreply@anthropic.com>
- Each sourced URL project gets an incrementing session id as a query parameter - Session IDs saved to IndexedDB; Db version migration included
a8d5a35 to
00eccd0
Compare
| /// Reflects a freshly minted session id in the address bar without pushing a | ||
| /// new history entry, so back navigation does not land on the bare URL. | ||
| void _writeSessionIdToUrl(Uri uri, int sessionId) { | ||
| final query = {...uri.queryParameters, sessionIdQueryParameter: '$sessionId'}; |
There was a problem hiding this comment.
queryParameters keeps only one value per key, but we support repeated file parameters. For example, opening ?sample=counter&file=a.dart&file=b.dart rewrites the URL with only one of those files. Reloading then changes the requested tabs.
Use queryParametersAll instead and add s.
There was a problem hiding this comment.
good catch, but would this mean that the same source with a different set of open files would count as a separate workspace? I think that what files are open should not dictate that, and only the source should. WDYT?
There was a problem hiding this comment.
No I would not count them separate, I agree. Just don't loose them on a refresh
1783af3 to
0be8bb3
Compare
0be8bb3 to
9092fb9
Compare
2 participants
In preparation for updated project persistance #3977: