Skip to content
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

Add remote asset listing to build #1502

Merged
merged 2 commits into from
Jun 10, 2024

Conversation

brandonpayton
Copy link
Member

Motivation for the change, related issues

This PR adds a remote asset listing to minified WP builds so we can later tell which files are expected to exist remotely and which should be considered missing if they are not present locally.

This was originally part of request routing PRs #1417 and #1490, but since there are some sensitive edge cases around the routing changes and browser storage, I am breaking the build changes into their own PR so the more sensitive changes can be reviewed more easily on their own.

Implementation details

This PR updates the minified WP build process to generate a wordpress-remote-asset-paths file containing the WP-relative paths of all assets not included in the minified build. That way, we have the necessary information to judge whether a requested resource can be requested from playground.wordpress.net when it does not exist locally.

We include this listing with new minified WP builds.

In case browser storage has already cached a minified WP build without this listing, we also make it available remotely so it can be retrieved as needed.

Testing Instructions (or ideally a Blueprint)

  • CI tests

@brandonpayton
Copy link
Member Author

@adamziel @bgrgicak would there be any value in creating a more generic wordpress-build-meta.json file or something similar and include the listing in that? In some of our boot-related discussions, I've felt like it might make sense to maintain such a file for these "special" minified builds, but I do not currently recall the use case.

For now, since we have discussing the listing file before, I am going to treat this change as non-controversial and merge it so the request routing changes can build on it.

@brandonpayton brandonpayton merged commit 17f166c into trunk Jun 10, 2024
5 checks passed
@brandonpayton brandonpayton deleted the add-remote-asset-listing-to-build branch June 10, 2024 18:34
@bgrgicak
Copy link
Collaborator

would there be any value in creating a more generic wordpress-build-meta.json file or something similar and include the listing in that?

What would the file include?
Having the current list of remote files seems good enough to me.

@adamziel
Copy link
Collaborator

@brandonpayton Keeping it separate is nice because we can access that list before unzipping WordPress or without stream-processing the ZIP file in JavaScript. Keeping it bundled in the zip would be nice because it would be harder for the two to get out of sync, e.g. due to HTTP caching. Either way seems fine.

@brandonpayton
Copy link
Member Author

@brandonpayton Keeping it separate is nice because we can access that list before unzipping WordPress or without stream-processing the ZIP file in JavaScript. Keeping it bundled in the zip would be nice because it would be harder for the two to get out of sync, e.g. due to HTTP caching. Either way seems fine.

This PR actually did both. :) The listing is included in the minified WP zips but available separately in case it needs to be downloaded on demand.

@brandonpayton
Copy link
Member Author

brandonpayton commented Jun 11, 2024

What would the file include?
Having the current list of remote files seems good enough to me.

@bgrgicak I was thinking we might be able to tag builds with a hash or build process version to detect when an older build has been cached to give an opportunity to update it. I think there may have been another possibility in my imagination but do not recall it today. We can drop it for now. Thanks!

@bgrgicak
Copy link
Collaborator

I was thinking we might be able to tag builds with a hash or build process version to detect when an older build has been cached to give an opportunity to update it.

This are both pieces that I'm missing for offline support . I plan to work on them this week.

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