Skip to content

Conversation

@adamziel
Copy link
Collaborator

@adamziel adamziel commented Jun 14, 2023

Since #547 adds support for OPFS API, I just realized the same API can be used to read/write local files. This PR adds a local directory picker and a "refresh" button to bring the changes back into Playground's MEMFS.

CleanShot 2023-06-15 at 19 37 10@2x

Testing instructions

  • Select an empty local directory
  • Confirm it populated that with files
  • Make a change to the homepage
  • Refresh the page
  • Confirm you got asked about whether to reuse that directory
  • Say yes
  • Confirm your homepage change is now visible

Other considerations

  • You can only save and load a Playground instance. The "context detection" logic from wp-now could be really handy here. It would have to be refactored to use shared, isomorphic feature that works through php methods like php.readFileAsBuffer() which means they'll need some enhancements like support for reading the first 8kb.
  • A proper OPFS filesystem for Emscripten would remove the need for a refresh files button as everything would be stored and read directly from OPFS. Persistent Playground: Explore OPFS support #544 started exploring that but it uses the deprecated sync API. We'd need to find a way to use the new async one.
    • On the flip side, a proper OPFS is also quite slow. Maybe the sync button is for the best?

@adamziel adamziel changed the title Support mounting local files playground.wordpress.net: Support mounting local files Jun 14, 2023
@adamziel adamziel marked this pull request as draft June 14, 2023 14:45
@adamziel adamziel marked this pull request as ready for review June 15, 2023 17:49
@adamziel adamziel force-pushed the mount-local-directory branch from e4b1bd7 to 58b4b4b Compare June 15, 2023 17:49
dmsnell added a commit that referenced this pull request Jun 16, 2023
When mounting local directories in #548, problems appeared in development
builds because the dev environemnt runs a separate server for the website
that wraps the iframe and the for the assets inside the iframe, the "remote."

In this patch we're adjusting the config for the outer website server so that
we can proxy requests for the internal frame through the same origin. This is
done by adding a prefix to the URL for the dev assets on the outer frame, which
then is used in a proxy rule to decide what to route.

This means that OPFS handles can be shared between the two sides of the iframe,
which normally happens in production since they both come from the same origin.
dmsnell added a commit that referenced this pull request Jun 16, 2023
When mounting local directories in #548, problems appeared in development
builds because the dev environemnt runs a separate server for the website
that wraps the iframe and the for the assets inside the iframe, the "remote."

In this patch we're adjusting the config for the outer website server so that
we can proxy requests for the internal frame through the same origin. This is
done by adding a prefix to the URL for the dev assets on the outer frame, which
then is used in a proxy rule to decide what to route.

This means that OPFS handles can be shared between the two sides of the iframe,
which normally happens in production since they both come from the same origin.
@dmsnell dmsnell requested a review from a team as a code owner June 16, 2023 17:07
adamziel pushed a commit that referenced this pull request Jun 18, 2023
When mounting local directories in #548, problems appeared in development
builds because the dev environemnt runs a separate server for the website
that wraps the iframe and the for the assets inside the iframe, the "remote."

In this patch we're adjusting the config for the outer website server so that
we can proxy requests for the internal frame through the same origin. This is
done by adding a prefix to the URL for the dev assets on the outer frame, which
then is used in a proxy rule to decide what to route.

This means that OPFS handles can be shared between the two sides of the iframe,
which normally happens in production since they both come from the same origin.
adamziel pushed a commit that referenced this pull request Jun 18, 2023
When mounting local directories in #548, problems appeared in development
builds because the dev environemnt runs a separate server for the website
that wraps the iframe and the for the assets inside the iframe, the "remote."

In this patch we're adjusting the config for the outer website server so that
we can proxy requests for the internal frame through the same origin. This is
done by adding a prefix to the URL for the dev assets on the outer frame, which
then is used in a proxy rule to decide what to route.

This means that OPFS handles can be shared between the two sides of the iframe,
which normally happens in production since they both come from the same origin.
adamziel added a commit that referenced this pull request Jun 18, 2023
When mounting local directories in #548, problems appeared in
development builds because the dev environemnt runs a separate server
for the website that wraps the iframe and the for the assets inside the
iframe, the "remote."

In this patch we're adjusting the config for the outer website server so
that we can proxy requests for the internal frame through the same
origin. This is done by adding a prefix to the URL for the dev assets on
the outer frame, which then is used in a proxy rule to decide what to
route.

This means that OPFS handles can be shared between the two sides of the
iframe, which normally happens in production since they both come from
the same origin.

Co-authored-by: Adam Zieliński <[email protected]>
adamziel added a commit that referenced this pull request Jun 18, 2023
When mounting local directories in #548, problems appeared in
development builds because the dev environemnt runs a separate server
for the website that wraps the iframe and the for the assets inside the
iframe, the "remote."

In this patch we're adjusting the config for the outer website server so
that we can proxy requests for the internal frame through the same
origin. This is done by adding a prefix to the URL for the dev assets on
the outer frame, which then is used in a proxy rule to decide what to
route.

This means that OPFS handles can be shared between the two sides of the
iframe, which normally happens in production since they both come from
the same origin.

Co-authored-by: Adam Zieliński <[email protected]>
@adamziel
Copy link
Collaborator Author

adamziel commented Jun 18, 2023

This now works and even stores the directory handle between page refreshes. Caveat: that handle can only be restored in a click event handler. To remedy that I added an open-by-default modal when storage=local is set and we have a stored directory handle:

CleanShot 2023-06-19 at 00 36 10@2x

The wording could be better. Also, React components code got a bit messy in the process and could use some cleaning up.

@adamziel adamziel force-pushed the mount-local-directory branch from 2ae3869 to c97c9d6 Compare June 18, 2023 22:38
@adamziel adamziel mentioned this pull request Jun 19, 2023
@dmsnell
Copy link
Member

dmsnell commented Jun 19, 2023

To remedy that I added an open-by-default modal when storage=local is set and we have a stored directory handle:

This is somewhat unfair, and even though the X exists on the dialog, we need a second button that effectively communicates "ignore the old session in build and start fresh."

Why?

Because we're asking "do you want?" and the only possible choice we give someone is "Yes."

@adamziel adamziel force-pushed the mount-local-directory branch from b871c70 to 18b2f35 Compare June 22, 2023 14:53
@adamziel adamziel force-pushed the mount-local-directory branch from 18b2f35 to d48991f Compare June 22, 2023 15:36
@adamziel
Copy link
Collaborator Author

Here's the updated modal:

CleanShot 2023-06-22 at 17 59 14@2x

@adamziel
Copy link
Collaborator Author

I've also learned the MEMFS->OPFS sync is so slow because Chrome writes everything to a temp file, scans it, and only then creates the actual file. There is chromium work underway to make it faster in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants