Skip to content
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

Closed
wants to merge 3 commits into from

Conversation

nataliecarey
Copy link

No description provided.


## 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.
Copy link
Contributor

@NickColley NickColley Sep 20, 2018

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?

  1. npm install hmrc-frontend
  2. 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

Copy link
Author

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.
Copy link
Contributor

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.

Copy link
Author

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.

@NickColley
Copy link
Contributor

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.

@NickColley
Copy link
Contributor

We met up and discussed the future of this proposal and the responsibilities for both sides and decided:

  • HMRC to lead initial implementation and guidance with GOV.UK Design System teams’ support
  • HMRC agreed with GOV.UK Design System team to complete planned work for refactoring the GOV.UK Prototype Kit before adding this proposal.
  • New scenarios will be added as challenges to the existing proposal which we’ll work through together to ensure the proposal meets them.

@joelanman has agreed to update this with the new scenarios

@penx
Copy link
Contributor

penx commented Dec 7, 2018

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 -

  • a home office design system extension system is created that can act as a prototype kit extension but also contains production ready code
  • similarly, an hmrc design system extension system is also created with production ready code
  • a site is created using a few components from govuk-frontend, 3 components from the home office design system and 1 component from the hmrc design system

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:

@penx
Copy link
Contributor

penx commented Jan 24, 2019

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:

@penx
Copy link
Contributor

penx commented Jan 24, 2019

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"}],

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

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

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

nataliecarey pushed a commit to dorightdigital/govuk-frontend that referenced this pull request Feb 7, 2019
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
36degrees pushed a commit to alphagov/govuk-frontend that referenced this pull request Mar 4, 2019
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
Copy link
Contributor

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

@36degrees
Copy link
Contributor

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.

@36degrees 36degrees closed this May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants