-
Notifications
You must be signed in to change notification settings - Fork 74
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
Expand and reorganise webR virtual filesystem API #127
Conversation
}, | ||
unlink: async (path: string): Promise<void> => { | ||
const payload = await this.#chan.request({ type: 'unlink', data: { path } }); | ||
if (payload.payloadType === 'err') { |
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.
There is now quite a lot of repeated code to check for WebRPayloadErr
and re-throw the error. We might be able to mitigate the repetition as part of a restructure of WebRPayload
into classes.
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.
For the moment I've extracted the repeated Error
construction into a new webRPayloadError()
function.
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 only took a quick look but LGTM.
When working on this part, please don't forget : https://github.com/georgestagg/webR/issues/56 |
For webR in particular a persistent VFS is tricker than at first glance. We cannot immediately use solutions that work well for other WASM projects because we use a blocking worker thread to maintain our place in the stack while waiting for input for R. Due to this we cannot easily yield to the event loop in the worker thread[1]. From what I currently understand there are several options available, but each have their own drawbacks for us,
I think the future of persistent storage in webR is going to be based around synchronous OPFS or wasmFS API, once they are ready and supported by more browsers. In the meantime, solving #56 would probably involve significant new infrastructure based around passing messages to the main thread to persist data. That amount of work should wait until after the first tagged version of webR has been released. [1] Asyncify would allow us to yield, but it does not work well for us due to the way the R interpreter works and the large asyncify overhead induced. |
Well understood! Important for us is to have this persistent file storage in the future! |
* Add `FS` namespace. * Rename functions to match Emscripten FS API. * Add `mkdir`, `rmdir` and `unlink` functions.
1433df7
to
06cfa71
Compare
Requires #126.
Expands and reorganises the webR filesystem API,
FS
namespace.mkdir
,rmdir
, andunlink
functions.