alphagov / govuk-frontend

GOV.UK Frontend contains the code you need to start building a user interface for government platforms and services.
https://frontend.design-system.service.gov.uk/
MIT License
1.16k stars 320 forks source link

The Prototype Kit has a hardcoded reference to the assets folder which has moved in v5 #3759

Closed 36degrees closed 1 year ago

36degrees commented 1 year ago

What

The changes to our repo structure in #3498 mean that the structure of the published package will change in v5. Most files have moved inside a top-level dist folder. For example, the favicon has moved from govuk/assets/images/favicon.ico todist/govuk/assets/images/favicon.ico.

However, the govuk-branded layout in the Prototype Kit includes a hardcoded assetPath that references the old assets location.

Work with the prototype team to support passing the asset path as part of the Prototype Kit config file.

Why

We need a way for the Prototype Kit to know where to find the assets in v5, whilst still allowing it to work with older releases of GOV.UK Frontend.

Who needs to work on this

Developers, Developers from the Prototype team

Who needs to review this

Developers, Developers from the Prototype team

Done when

36degrees commented 1 year ago

There's a suggested approach from @nataliecarey where the Prototype Kit adds support for a new top-level constants option which we can hook into:

 "constants": [
    {
      "name": "govukAssetPath",
      "value": "/plugin-assets/govuk-frontend/dist/govuk/assets"
    }
  ]

There's a branch of the Prototype Kit with the proposed changes we can use for testing:

mkdir kit-integration-demo
cd kit-integration-demo
npx govuk-prototype-kit create --version=alphagov/govuk-prototype-kit#frontend-v5-compatibility
npm install alphagov/govuk-frontend#pre-release-delete-package-dist # or npm link

We'll need the Prototype team to confirm they're happy with this approach and update the kit before we merge any changes on our side.

She has also flagged that in her testing the font CSS path was wrong, which can be fixed by adding /dist to the asset path in init.scss.

colinrotherham commented 1 year ago

Linking this alternative idea for require.resolve() which locates the assets path (same code) for both v4 and v5

For ES modules we also have import.meta.resolve() but it needs --experimental-import-meta-resolve in Node.js so would require us to continue providing CommonJS (or UMD) support

romaricpascal commented 1 year ago

@colinrotherham Sorry, I stepped a bit on this piece of work. To facilitate discussions for the Prototype Kit team, I quickly spiked the approach described by Ollie in spike-prototype-kit-constant to check that it did indeed solve the 404 when used with @nataliecarey's branch, which it does đŸ¥³. It's just a spike branch, with preview-spike-prototype-kit-constant so very throwaway, just to validate things work.

romaricpascal commented 1 year ago

Also a few thoughts from that quick spike:

colinrotherham commented 1 year ago

@colinrotherham Sorry, I stepped a bit on this piece of work.

Thanks @romaricpascal, no that's great

Here's the part I've been working on, running the Prototype Kit with our local GOV.UK Frontend:

colinrotherham commented 1 year ago

I've been working on path changes in this PR:

@romaricpascal Your comments regarding /plugin-assets/govuk-frontend were right. In the Sass for example, we'd receive this path suffix using $govuk-extensions-url-context in init.scss which the kit provides

E.g. Prototype management pages use /manage-prototype/dependencies/govuk-frontend

GOV.UK Frontend paths

The following paths are used currently in various places:

  1. Base path: node_modules/govuk-frontend
  2. Entry path: node_modules/govuk-frontend/govuk/all.js
  3. Include path: node_modules/govuk-frontend/govuk
colinrotherham commented 1 year ago

PR ready for review

Some tests are failing because scripts aren't waiting for the GitHub preview to finish installing

colinrotherham commented 1 year ago

Happy to say this is now closed in https://github.com/alphagov/govuk-prototype-kit/pull/2306

The Prototype Kit supports govuk-frontend@4.0.0 so backporting new config fields wasn't feasible

Instead we simply join /assets to the directory of our main package export (as shown above)

const { dirname, join } = require('path')

// `govuk-frontend@4` /path/to/project/node_modules/govuk-frontend/govuk/assets
// `govuk-frontend@5` /path/to/project/node_modules/govuk-frontend/dist/govuk/assets

const assetsPath = join(dirname(require.resolve('govuk-frontend')), 'assets')