Use USE_LOCAL_ASSETS env var to determine path for assets#16256
Open
jonathonherbert wants to merge 6 commits into
Open
Use USE_LOCAL_ASSETS env var to determine path for assets#16256jonathonherbert wants to merge 6 commits into
jonathonherbert wants to merge 6 commits into
Conversation
|
Hello 👋! When you're ready to run Chromatic, please apply the You will need to reapply the label each time you want to run Chromatic. |
3360067 to
0c9e5ce
Compare
…ht GHA, to be more explicit about where local assets are necessary, and what drives the decision to use them
106d749 to
0e07458
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this change?
Amend
decidePublicPathto use the environment variableUSE_LOCAL_ASSETSto determine the path for assets, rather than a combination of theNODE_ENVvariable, and the hostname.Why?
The hostname for using live harnesses to work on content atoms locally (docs) is always localhost. At the moment,
decidePublicPathsees that hostname, and makes asset paths relative as a result — but because DCR isn't running locally, those paths don't resolve to assets. The hope is that this change accommodates both the workflows that require local assets (make dev, make prod-local, CI), and the workflows that require hosted assets (code, production, live harnesses).Here's a table that maps out the old way, the new way, and the result, against all the workflows we know of (I've spoken to @arelra and @Jakeii). If other workflows exist, this PR may impact them, too — so please let me know if there's a row missing.
/assets//assets//assets/${frontendAssetsFullURL}assets/${frontendAssetsFullURL}assets/How to test
make dev) with frontend gives local asset pathsmake prod-local) with frontend gives local asset pathsScreenshots
N/A