Fix error server /manage-prototype/dependencies#2333
Conversation
|
To confirm it works, this error is back on Manage prototype pages: The kit must be running It's fixed on plugin pages which appear to use |
/manage-prototype/dependencies assets/manage-prototype/dependencies
|
Historically we've tested the isolation by trying scenarios where:
The case that's most relevant to supporting future versions of Frontend hasn't historically been tested. The way I'm testing this today is:
That will install a completely blank In that scenario the kit fails to start both in |
Happy to accept a challenge, but some of your tests need Is running without GOV.UK Frontend entirely more for this one? |
283a82b to
0832c38
Compare
GOV.UK Frontend is no longer served from `/plugin-assets` on the error page but we didn’t handle `/manage-prototype/dependencies`
Ensures GOV.UK Frontend fonts, images, favicons etc are loaded correctly
This gives internal pages their own `$govuk-assets-path` Sass variable to internal GOV.UK Frontend to align with Nunjucks Whilst plugin Sass can continue to use `/plugin-assets`
Whilst not strictly necessary, packaged Sass files in GOV.UK Frontend are processed by PostCSS + Autoprefixer so we should reflect this when inspecting original sources in the browser
0832c38 to
bb7ec01
Compare
|
Thanks @nataliecarey Just a reminder that you'll now see GOV.UK Frontend v4.5.0 internally due to govuk-prototype-kit/package.json Line 75 in 86fb72a Prototype pages using plugin assets won't be affected Would be good to do another PR to avoid the source map 404 issue #2333 (comment) |
GOV.UK Frontend is now only loaded from `/manage-prototype/dependencies` on internal pages
| @@ -1,3 +1,4 @@ | |||
| {%- set assetPath = '/manage-prototype/dependencies/govuk-frontend/govuk/assets' -%} | |||
There was a problem hiding this comment.
Just taking another look. We've got a layout we're using for Manage Prototype section which I think we should be using here, that layout sets assetPath as you are here so I'd suggest we remove the explicit assetPath and extend views/manage-prototype/layout.njk instead.
You can see this on the 404 page, I must have missed the error page when I was going through the internal pages.
There was a problem hiding this comment.
Sounds good to me. Pushed it up 👍
There was a problem hiding this comment.
@nataliecarey Going to push again as the new layout changes application.css to manage-prototype.css
There was a knownPaths entry that expected application.css
const knownPaths = {
'/public/stylesheets/application.css': {
type: 'text/css',
contents: fs.readFileSync(path.join(process.cwd(), '.tmp', 'public', 'stylesheets', 'manage-prototype.css'))
}
}f7d16cc to
4c82ead
Compare
This PR fixes a few error page issues
It picks up from changes to isolate GOV.UK Frontend versions in:
Sass variables
The error page Sass
$govuk-assets-pathuses /manage-prototype/dependencies/govuk-frontend but we didn't handle error server responses for this route so fonts and background images were missing:govuk-prototype-kit/lib/assets/sass/manage-prototype.scss
Line 1 in 86fb72a
Nunjucks assets
The error page Nunjucks
assetPathuses /plugin-assets/govuk-frontend which would typically route to a plugin version of GOV.UK Frontend rather than one internal to the Prototype KitThis was handled but with different URLs to the CSS:
govuk-prototype-kit/lib/errorServer.js
Lines 59 to 61 in d262239
Sass variables
The error page shares the
manage-prototype.scssstylesheet which is hard coded to use:govuk-prototype-kit/lib/assets/sass/manage-prototype.scss
Line 1 in 86fb72a
HTTP content-type headers
GOV.UK Frontend includes fonts (
*.woff,*.woff2), images (*.png,*.svg) and favicons (*.ico)These have now been added to the error server including
*.mjswith the old-but-newtext/javascripttypegovuk-prototype-kit/lib/errorServer.js
Lines 27 to 30 in 86fb72a
Legacy GOV.UK Frontend
The Prototype Kit currently patches versions of GOV.UK Frontend prior to v4.4.0 (where
nunjucksMacrosandsassfields were added to the plugin config) but the$govuk-assets-pathvariable was missing leaving the default as: