Support Prototype Kit v13

[nataliecarey]

Includes:

• Using the new plugin interface to define component includes.

[36degrees]

Thanks for doing this! Looks good, my only comment / suggestion is re adding some tests

[36degrees]

If we can't generate this programatically, it might be worth adding something to /tasks/gulp/__tests__/after-build-package.test.js to check that all components are in this list, and that the referenced file looks correct and exists.

That'll then hopefully prevent us from forgetting to e.g. add new components to the list.

[nataliecarey]

That's fair. If that can be picked up by automated tests then it can be generated automatically too :)

[nataliecarey]

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:

1. The initAll() line has been moved out of the prototype kit
2. Frontend now uses the standard asset path for extensions
3. 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:

1. Add to their routes.js:

require('govuk-prototype-kit').requests.serveDirectory('/assets',
  'node_modules/govuk-frontend/govuk/assets')

2. Add to their application.js:

window.GOVUKPrototypeKit.documentReady(() => {
  window.GOVUKFrontend.initAll()
})

3. Include the {% from ... import ... %} line when using components from the design system

4. 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.

[36degrees]

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:

govuk-frontend/tasks/gulp/copy-to-destination.js

Lines 112 to 119 in 64418c8

	gulp.task('js:copy-esm', () => {
	return gulp.src([
	`${configPaths.src}**/*.mjs`,
	`${configPaths.src}**/*.js`,
	`!${configPaths.src}**/*.test.*`
	])
	.pipe(gulp.dest(taskArguments.destination + '/govuk-esm/'))
	})

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…

[colinrotherham]

So I think we can just remove the ${configPaths.src}**/*.js glob from the js:copy-esm task?

@nataliecarey @36degrees Just a comment to say this was picked up in #2876 to avoid Gulp change conflicts

[36degrees]

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 nunjucksMacros array

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 #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.

[colinrotherham]

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

[romaricpascal]

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.

[nataliecarey]

I'm not impacted by the outcome, I'm just saying this to help if I can.

1. 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.
2. It could make sense to have a govuk-prototype-kit directory with the config file, the JS and the SCSS - everything that's specifically for the kit.
3. 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?

[36degrees]

I'm not impacted by the outcome, I'm just saying this to help if I can.

1. 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.

2. It could make sense to have a govuk-prototype-kit directory 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 👍🏻

3. 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.

[nataliecarey]

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 :)

[36degrees]

These should be !default so that users can opt out in the Prototype Kit settings file.

[colinrotherham]

@nataliecarey I've rebased your branch just to make it a bit easier to make the fixes on our side

[36degrees]

Approach looks sound to me, but I think we can rely on the existing tests for the 'after build tests' given we're only checking the files exist anyway.

(I think it'd be neater if the .js and .scss files were handled by the existing copy-files task, but that looks like it's caught up in the fact that config.src is actually src/govuk so happy to revisit that later)

[36degrees]

These are already covered by the 'contains the expected files' test which:

• looks for a matching file for everything in the src/ directory (apart from the excluded files), and so already expects corresponding files in the package for govuk-prototype-kit/init.js and govuk-prototype-kit/init.scss
• includes govuk-prototype-kit.config.json in the list of 'extra' files

[colinrotherham]

Fab, just me being overly cautious. I'll remove 👍

[colinrotherham]

@36degrees These are gone now

I've also swapped the "already present" govuk-prototype-kit.config.json with mapped replacement:

// Replaces GOV.UK Prototype kit config with JSON
.flatMap(mapPathTo(['**/govuk-prototype-kit.config.mjs'], ({ dir: requirePath, name }) => [
  join(requirePath, '../', `${name}.json`)
]))

Similar to the YAML file transform, it shows the config is replaced with JSON a directory up (same name)

[lfdebrux]

🙌 Amazing! Thanks so much all 😁