Use USE_LOCAL_ASSETS env var to determine path for assets#16256
Use USE_LOCAL_ASSETS env var to determine path for assets#16256jonathonherbert wants to merge 12 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
| env: | ||
| NODE_ENV: production | ||
| NODE_ENV: 'production' | ||
| USE_LOCAL_ASSETS: 'true' |
There was a problem hiding this comment.
I think the NODE_ENV variable here determines which port playwright uses i.e. 9000
dotcom-rendering/dotcom-rendering/playwright.config.ts
Lines 3 to 9 in ba85b8b
I don't think these env variables on the GHA runner will make it to the container that the CI playwright tests run against?
Do we need to add USE_LOCAL_ASSETS=true to Containerfile ?
dotcom-rendering/dotcom-rendering/Containerfile
Lines 12 to 13 in ba85b8b
There was a problem hiding this comment.
Ah, we don't use the container in production — so it's safe to add this?
Edit: the bundle doesn't look like it's build in the container, it's copied in, so we'll need to pass this in on build, if that's possible — run out of time tonight, but will pursue.
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//assets//assets//assets/${frontendAssetsFullURL}assets/${frontendAssetsFullURL}assets/${frontendAssetsFullURL}assets/${frontendAssetsFullURL}assets//assets/${frontendAssetsFullURL}assets/How to test
make dev) with frontend gives local asset pathsmake prod-local) with frontend gives local asset pathsScreenshots
N/A