-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add an extension system for the prototype kit. #9
Conversation
|
||
## Proposed Implementation | ||
|
||
Any node module which is in the package.json file containing a govuk-prototype-kit.config.json file in its root directory will be processed as an extension. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In response to how the package itself exposes it's settings, this could in theory be within the published package.json of the package?
// hmrc-frontend/package.json
{
"govuk-prototype-kit-extension-config": {
...
}
}
In response to 'Specifying the order of extensions', have you considered a plugin system like babel?
- npm install hmrc-frontend
- add reference to package.json (order is then controlled here)
{
// govuk-prototype-kit/package.json
"govuk-prototype-kit": {
"plugins": [
"govuk-frontend", // aliases automagically to the pre-installed govuk-prototype-kit-govuk-frontend package
"hmrc-frontend"
]
}
}
Inspired by https://babeljs.io/docs/en/babel-preset-latest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In an older version we had the order controlled in the package JSON, we decided that config.js was a less intimidating place for this, partly so we could add comments.
One thing I feel is very important is that an NPM install is enough to start using a new extension which doesn't conflict with existing extensions. If everyone's producing namespaced extensions then no prototype kit user will ever need to edit the order.
|
||
By including a [configuration file](https://github.com/hmcts/frontend/issues/31) HMCTS and HMRC components can be installed via npm and then used immediately without prototype users needing to make changes to source files. | ||
|
||
Govuk-frontend could be integrated in the same way, this both proves that the extension system is powerful enough to support govuk’s needs and removes hard-coded references to the govuk-frontend module. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this part of the proposal, as if we decide to do this it'll force us to test and maintain this extension system properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent - I totally agree.
If we can assume that govuk-frontend
will contain a config file then it simplifies the PR I'm ready to raise to govuk-prototype-kit
. Simplification of the code and "dogfooding" is good IMHO.
An update for those following this: The GOV.UK Design System team is excited about this proposal and wants to take this forward, we have some minor questions around this specific implementation. We intend on catching up with Mat and the team in person to work through this together. |
We met up and discussed the future of this proposal and the responsibilities for both sides and decided:
@joelanman has agreed to update this with the new scenarios |
Once this extension system exists, I can see people starting to use it for production code. Do we want to think about ways we could cater for this, or dissuade people from doing so all together? An example -
My concern with this would be that the entire CSS from all 3 would be pulled in even though only a subset is being used. My recommendation is to recommend extensions are compatible with CSS modules so that just a subset of the code could be used and enabling tree shaking to remove redundant code. Related:
|
to aid porting extensions to templating languages other than nunjucks (e.g. React such as the PoC here https://github.com/penx/govuk-frontend-react ) it would also be useful for Jest tests to be provided in the extensions in a way that can be used by ports. I have made some progress on this for the base govuk-frontend tests. See: |
of particular value to us, if this goes ahead, would be a home office design system extension and an admin-controls extension, which we could then look in to porting to React using a similar approach to govuk-frontend-react |
{ | ||
"nunjucksPaths": ["/", "/components"], | ||
"scripts": ["/all.js"], | ||
"assets": [{"global": true, "src": "/assets"}], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be updated inline with what is in govuk-frontend
|
||
### Specifying the order of extensions | ||
|
||
We expect conflicts between extensions to be very rare but when they occur it would be good for the prototype user to have a solution available. In the proposed implementation this is done by editing `foundationExtensions` in `app/config.js`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to update foundationExtensions
to baseExtensions
|
||
As different technologies deal with overrides in different ways (for Nunjucks the first takes priority, in CSS the last takes priority) the extension priorities are normalised so that later extensions will always override the earlier ones. The order will be: | ||
|
||
First: The installed modules listed in foundationExtensions in the order specified |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to update foundationExtensions
to baseExtensions
To accompany the extensions proposal this config allows govuk-frontend to be an extension in the prototype kit. Implementation details can be found here alphagov/govuk-prototype-kit#613 Proposal details can be found here alphagov/govuk-design-system-architecture#9
To accompany the extensions proposal this config allows govuk-frontend to be an extension in the prototype kit. Implementation details can be found here alphagov/govuk-prototype-kit#613 Proposal details can be found here alphagov/govuk-design-system-architecture#9
@@ -0,0 +1,84 @@ | |||
# GOVUK Prototype Extensions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will need to rename this from 004
when we merge this... @matcarey have you got time to get this ready to merge, or do you want us to clean it up? :)
I'm closing this as it's somewhat out of date at this point (!) and the Prototype Kit has split out from the Design System, so this doesn't seem the right place to record this decision any more. |
No description provided.