govuk-frontend
govuk-frontend copied to clipboard
Supporting v13 of the Prototype Kit
Includes:
- Using the new plugin interface to define component includes.
Could someone from the Design System Team take a look at the test failure here please? I tried to fix it locally before pushing but I'm not clear on the target output and the mechanism of the test itself. It's failing because of the addition of a Javascript file, I'm not sure whether I've put the file in the right location or not.
I've tested this latest update with different versions of the prototype kit, it works against versions 11-13 (and I have no reason to believe it would fail in any version which uses extensions).
There's a few things that have changed in v13 which are supported with this change:
- The
initAll()line has been moved out of the prototype kit - Frontend now uses the standard asset path for extensions
- The components can be auto-loaded based on this configuration but govuk-frontend components aren't auto-loaded without this configuration
I've included a window.GOVUKPrototypeKit.majorVersion variable in the Javascript and a $govuk-prototype-kit-major-version variable in the SASS. These enable the conditional logic needed to allow users to use a new version of frontend with an older version of the kit if they're not ready to upgrade the kit yet.
If these things aren't provided by govuk-frontend or the user uses an older version of govuk-frontend without it then they'll have to:
- Add to their
routes.js:
require('govuk-prototype-kit').requests.serveDirectory('/assets',
'node_modules/govuk-frontend/govuk/assets')
- Add to their
application.js:
window.GOVUKPrototypeKit.documentReady(() => {
window.GOVUKFrontend.initAll()
})
-
Include the
{% from ... import ... %}line when using components from the design system -
Not be able to use global styles or new link style
The user can work around points 1-3 and we (the prototype kit team) should give them a way of working around point 4 but it would be great if we can release v13 of the kit containing a version of govuk-frontend which doesn't need setting up.
Could someone from the Design System Team take a look at the test failure here please? I tried to fix it locally before pushing but I'm not clear on the target output and the mechanism of the test itself. It's failing because of the addition of a Javascript file, I'm not sure whether I've put the file in the right location or not.
If I run npm run build:package I can see that the _govuk-prototype-kit-init.js file is being copied to two places – package/govuk/_govuk-prototype-kit-init.js and package/govuk-esm/_govuk-prototype-kit-init.js.
The govuk-esm folder exists to allow users to import GOV.UK Frontend JavaScript as ECMAScript (ES) modules.
I don't think it makes sense to include _govuk-prototype-kit-init.js in the ESM folder as it won't be relevant for those users.
Looking at the js:copy-esm task, it currently copies *.mjs and *.js files:
https://github.com/alphagov/govuk-frontend/blob/64418c826d3596f892ae9eac6608a9f3288fefe0/tasks/gulp/copy-to-destination.js#L112-L119
However, I think we probably only want to copy *.mjs. I don't think there are any *.js files that we do want to end up in govuk-esm:
➜ find src -name '*.js' -a '!' -name '*.test.js'
src/govuk/_govuk-prototype-kit-init.js
src/.eslintrc.js
So I think we can just remove the ${configPaths.src}**/*.js glob from the js:copy-esm task?
Would be good to get a second set of eyes on this from someone on the team to check I'm not missing something…
So I think we can just remove the
${configPaths.src}**/*.jsglob from thejs:copy-esmtask?
@nataliecarey @36degrees Just a comment to say this was picked up in https://github.com/alphagov/govuk-frontend/pull/2876 to avoid Gulp change conflicts
I think there are three things we need to decide on to unblock this:
- Where in the package the new CSS and JS files live
- Where we 'author' the new CSS and JS files
- Whether to automatically generate the
nunjucksMacrosarray
Where the new CSS and JS files live
For the purposes of separation, my preference would be to keep the new prototype-kit specific JS and CSS files outside of the govuk folder, either in the root or in a prototype-kit folder.
node_modules/govuk-frontend
├── govuk
│ ├── assets
│ ├── components
│ ├── core
│ └── …etc…
├── prototype-kit
│ ├── init.js
│ └── init.scss
└── govuk-prototype-kit.config.json
Where we author the new CSS and JS files
At the minute, the govuk-prototype-kit.config.json file only exists in the package folder.
So when the package is built, the clean task is configured to leave the config file in place, and the version in the package folder is not added or altered by any of our tasks.
(It's worked like that since it was added in https://github.com/alphagov/govuk-frontend/pull/1102 and I don't know how intentional this choice was)
Option 1: Config in package, CSS + JS in src
This is what we currently have. The config file only exists in the package folder, the CSS and JS are copied from src.
Feels slightly messy / inconsistent?
Option 2: Only add the new CSS and JS files to the package folder
This is consistent with what we're currently doing for the config file, but I'm not sure that's a precedent we want to follow.
We generally tell people not to edit files in the package folder. Although it's unlikely we'll need to change the file often, it feels cleaner to avoid this 'exception to the rule' if we can.
Option 3: Config, CSS and JS live in src
They'd be copied at build time, with any additional processing done as part of the build process (see the next section about dynamically generating parts of the config).
I think this is my preference at the minute.
Option 4: Separate package
We release a separate npm package for the kit which either includes or depends on govuk-frontend and includes the files needed for integration.
(I've included this for completeness, but don't think we want to do this as it introduces an extra level of dependency versioning that we just don't need at this time.)
Whether to automatically generate the nunjucksMacros array
I think this is tied to the previous question about where we author the files, as it introduces a generation / modification step as part of the build process – so it probably makes sense to have the 'source' in 'src'!
If we don't do this, we probably want to add tests to ensure that the nunjucksMacros array contains all of the expected components, to avoid a scenario where we add a new component but forget to add it to the array, which would mean it would not be loaded in the prototype kit.
However, as Nat has pointed out, if we can add tests for it then we are able to programatically know what macros should be in the array, and therefore it should be relatively easy to generate it.
I'd go with Option 3 and (controversially) delete ./package
Feels like we should be able to pull any commit, install, and build the generated files:
./node_modules
./public
./sassdoc
./package
./dist
Separating prototype kit specific code into its own folder sounds good to me, and I'd say that both config and JS and SCSS files should live there as well (option 3), even if we don't manipulate the config file and just copy a manually authored one across. That said, I'd be in favour of building the config file rather than authoring manually and adding a test.
I'm not impacted by the outcome, I'm just saying this to help if I can.
- Removing
package/would be good, it's difficult to avoid committing package files by accident and they're generated files anyway so removing them should be quite painless. - It could make sense to have a
govuk-prototype-kitdirectory with the config file, the JS and the SCSS - everything that's specifically for the kit. - Auto-generating could be pretty easy given how well ordered the components are.
Would someone like to pair on this over the next few working days?
I'm not impacted by the outcome, I'm just saying this to help if I can.
- Removing
package/would be good, it's difficult to avoid committing package files by accident and they're generated files anyway so removing them should be quite painless.
This is a big change and I see no reason to tie it to this work, so I'd like to consider this out of scope. If this is something we want to explore, we should consider it as part of the other future architectural changes for GOV.UK Frontend.
- It could make sense to have a
govuk-prototype-kitdirectory with the config file, the JS and the SCSS - everything that's specifically for the kit.
I think this would require a change to the Prototype Kit as I think it only looks in the top-level of each node module? If this is something you're open to I think having it all within a single folder would be good 👍🏻
- Auto-generating could be pretty easy given how well ordered the components are.
👍🏻
Would someone like to pair on this over the next few working days?
Pairing between the two teams sounds good, but I don't think I'm personally going to have capacity for this in the next few days – but think this is good for anyone to pick up?
I have raised an issue to track this work, added it to our sprint backlog and the milestone for the next release – @nataliecarey if you have a chance to review it and make sure it covers everything we need to do, that'd be great.
This is a big change and I see no reason to tie it to this work
Makes sense.
I think this would require a change to the Prototype Kit as I think it only looks in the top-level of each node module?
Sorry, I wasn't clear - I meant that you could put those files all together and the build job could move the config file into place.
Pairing between the two teams sounds good, but I don't think I'm personally going to have capacity for this in the next few days – but think this is good for anyone to pick up?
Anyone is welcome to reach out to me about pairing :)
@nataliecarey I've rebased your branch just to make it a bit easier to make the fixes on our side
@nataliecarey @36degrees I've moved the config to its new destination: https://github.com/alphagov/govuk-frontend/pull/2851/commits/9bfd2394ce3c9df2ac38c914a10e6878630c2147
Following the chat above it seems govuk-prototype-kit was preferred versus prototype-kit?
node_modules/govuk-frontend
├── govuk
│ ├── assets
│ ├── components
│ ├── core
│ └── …etc…
├── govuk-prototype-kit
│ ├── init.js
│ └── init.scss
└── govuk-prototype-kit.config.json
@36degrees You might have wondered why the "Fix Windows" PR https://github.com/alphagov/govuk-frontend/pull/2946 is the new base for this one, just means we can grab our getDirectories() helper to build the config nunjucksMacros array automatically 🎉
Tests are failing currently as I'm just doing the build script to output govuk-prototype-kit.config.json
Tests added https://github.com/alphagov/govuk-frontend/pull/2851/commits/8d21c7f8eedbc1cc9075085ab55c5aaa92caea23 just need to output the files
🙌 Amazing! Thanks so much all 😁