-
Notifications
You must be signed in to change notification settings - Fork 350
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Race condition when programmatically updating files during recompile #1181
Comments
Just encountered the same problem. Before I also loaded the changes before loading Sandpack, but changed that this week so Sandpack would load quicker. Now I'm also running into the problem where the code won't update in the preview, and it still shows the empty template. So this bug still exsits. Did you happen to find any other solution rather than your workaround @a-type? |
Afraid not. We've since migrated off Sandpack proper and onto the static file serviceworker instead (https://sandpack.codesandbox.io/docs/advanced-usage/static) since we didn't need anything beyond vanilla JS. Notably, we experienced that my 'workaround' only minimizes chances of race conditions, it doesn't eliminate them. They still cropped up every once in a while if the preloaded source files change too quickly (for example, if the user types in the editor immediately on page load, or if the files are refreshed automatically for some reason shortly after loading). I'm pretty confident this will require a change to the code I pointed to, but it shouldn't be particularly complicated (basically, debounce instead of throttle). |
Thanks for letting me know! I've looked at multiple workarounds but all of them feel a bit hacky, and indeed might fail in some edge cases too. Currently looking into alternative playgrounds that might be out there, although that's a big rewrite. This all started out as a very tiny change I wanted to do, until I stumbled on this issue and lost almost a week of my time 😅 Which is why I'm now doubting whether it's worth it trying to fix it, or I can better spend time rewriting it entirely. Anyway, appreciate you getting back to me. |
That kinda sounds like my experience too. Sandpack rocks until you hit a snag, at which point it's hard to recover. Not saying that negatively; that's the case with any high-level abstraction over a very complicated system! Not sure of your needs, but since we didn't really need transpilation/bundling, I was actually able to entirely replace Sandpack on volu.dev with |
Interesting! I decided to take a different approach here. I’m now using PrismJS as the editor, and then only use the preview part of sandpack to send the files to. Works flawlessly — and I was able to convert to it in under a day. Might need to replace the preview one day too. But this way I also didn’t need to worry about the compiling at all, since that is still done by sandpack, and that part was still working perfect for me. |
It's possible you'll still hit the race condition if you send files to Sandpack too quickly after initialization / last change. But I hope it works! We just added transpiling to our handspun editor, it was pretty easy with the WASM build of esbuild. External module dependencies are handled with https://esm.sh and a dynamic |
Bug report
Packages affected
Description of the problem
I'm using Sandpack to bundle and run pre-existing code which is controlled by users and stored externally. The current approach involves loading these files when the external file source is ready, overwriting an initial default empty template. This overwrite-load happens inside a
useEffect
very shortly after Sandpack is initialized.However, I've observed a race condition when doing this: in almost all cases, despite
updateFile
being called, Sandpack will ignore the new files and keep rendering the blank template upon startup.I believe the source of this race condition is in this logic, which ignores file changes if the client is actively recompiling. Since my file update happens shortly after startup, my theory is the client is busy compiling the empty template, and so the real files are simply never loaded in the client unless I trigger another change later on. But I'm not 100% sure this is the problem. My only evidence to support it is that if I set a logpoint on that line in my devtools while reloading the page, the only log I see shows that
filesState.files
has the correct (updated) file contents, butclient.status
is'initializing'
, meaning it will not be updated. The logpoint is only triggered once; so it seems that because the timing was unfortunate, it misses its only chance to load the new file contents.A minimal demonstration of this problem is shown in the codesandbox below. As you can see, the code in the editor has been properly updated, but the sandbox is out of date. Uncomment the timeout wrapping the
updateFile
call to see how a delay avoids the race condition:Sandbox
If this truly is a race condition related to the line of code cited above, I might suggest that if the client is busy, the file watching logic should re-enqueue the update for later. Alternatively, a more sophisticated approach might be to cancel current compilation if possible and restart with the latest code.
Workaround
For now I am refactoring my app to fully load external sources before initializing Sandpack, so that instead of starting from a blank template and replacing its files once ready, the app instead waits until it can use the external file sources as the initial files in Sandpack setup. This is probably preferable anyway, but I felt the race condition still feels 'wrong' enough to report to you for consideration. There may be legitimate scenarios where this race condition is triggered either by rapid user input or a slow compilation.
The text was updated successfully, but these errors were encountered: