-
Notifications
You must be signed in to change notification settings - Fork 274
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
Support mutliple sites when using browser storage #1472
Conversation
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 left one note but LGTM otherwise
create: true, | ||
}); | ||
virtualOpfsDir = await virtualOpfsRoot.getDirectoryHandle( | ||
startupOptions.siteSlug, |
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.
Let's prefix that with something or, even better, put it in /sites/ directory. This way we'll be able to safely use the rest of OPFS without risking finding a WordPress site in a random location.
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 added prefixes. If I moved sites to a subfolder the default WordPress site would be special or I had to be moved, so prefixes seemed cleaner.
What is this PR doing?
It adds support for storing multiple sites in browser storage.
Up until this PR only one site could be stored in a browser. If you wanted to add another site, you would need to overwrite the first one. With this PR you are able to add more sites without affecting existing sites.
What problem is it solving?
There is no way to run multiple Playground sites with browser storage enabled because sites override each other.
How is the problem addressed?
By adding a
site-slug
query argument that allows users to specify which site to load.Testing Instructions