-
Notifications
You must be signed in to change notification settings - Fork 239
overhaul adapter code #249
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
/** @type {Promise<boolean>} */ | ||
let current; | ||
let token = {}; | ||
async function next() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The simplification inside Output.svelte
is great, but I think we need to keep this queue implementation. The new code is - I think - prone to race conditions when typing fast, toggling between solution and initial state fast etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't observed that to be the case, but if it is then I think we want to implement the queuing logic in webcontainer/index.js
, where there's already an await running
(which appears to only be used in the reset
case) — having multiple layers was part of the reason the code was hard to work with. As far as adapter.js
goes it should be fire-and-forget, I think
…gs are both written simultaneously
still occasionally encountering errors when navigating between apps, but no more than currently, so i'll merge this to unblock other work |
This simplifies the adapter code per #248. There was a bit too much indirection previously (also we were conflating state and events, which I think is a bit of a code smell) and I found it quite hard to make changes — there's still room for improvement but I'm much happier with this.
There are a couple of glitches to work through still:
/about
on /tutorial/pages, that path will persist when you navigate to other exercises