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

Locate GOV.UK Frontend using require.resolve() #2306

Merged
merged 4 commits into from
Sep 21, 2023

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Aug 22, 2023

This PR ensures we locate all previously hard-coded GOV.UK Frontend paths

This is in preparation for path changes between v4 (current) and v5 (upcoming) releases:

GOV.UK Frontend v4

  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

GOV.UK Frontend v5

  1. Base path: node_modules/govuk-frontend
  2. Entry path: node_modules/govuk-frontend/dist/govuk/all.bundle.js
  3. Include path: node_modules/govuk-frontend/dist/govuk

Partially closes #2293 and resolves alphagov/govuk-frontend#3759

@colinrotherham colinrotherham force-pushed the frontend-v5-compatibility-colin branch 3 times, most recently from a73b9d9 to 3e9f2f7 Compare August 22, 2023 14:15
lib/errorServer.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@colinrotherham colinrotherham force-pushed the frontend-v5-compatibility-colin branch 2 times, most recently from e01676d to 592ea50 Compare August 22, 2023 18:03
@colinrotherham
Copy link
Contributor Author

I've moved the GOV.UK Frontend v5 preview commits to another branch

Installing the preview from GitHub via npm install was causing test issues:

npm install --save "alphagov/govuk-frontend#6d1a4ef0a"

Not sure if a few sleep() or cy.wait() commands were expecting it to take so long?

Test output is looking a lot better now with some new changes pushed

@colinrotherham
Copy link
Contributor Author

Pushed again with the GOV.UK Frontend v5 preview

@colinrotherham colinrotherham force-pushed the frontend-v5-compatibility-colin branch 3 times, most recently from 237d632 to 6f55f04 Compare September 1, 2023 16:17
@colinrotherham colinrotherham changed the title Updates for GOV.UK Frontend v5.0.0 support Automatically locate GOV.UK Frontend paths Sep 1, 2023
package.json Outdated Show resolved Hide resolved
__tests__/spec/sanity-checks.js Outdated Show resolved Hide resolved
__tests__/spec/sanity-checks.js Outdated Show resolved Hide resolved
bin/utils/index.js Outdated Show resolved Hide resolved
lib/authentication.js Outdated Show resolved Hide resolved
lib/authentication.js Outdated Show resolved Hide resolved
lib/errorServer.js Outdated Show resolved Hide resolved
lib/errorServer.js Outdated Show resolved Hide resolved
server.js Outdated
Comment on lines 64 to 69
// Set up GOV.UK Frontend relative paths
app.locals.govukFrontend = {
assetPath: govukFrontend.assetPath,
scripts: govukFrontend.config?.scripts || []
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nataliecarey I think these lines need a suggestion

As written, Nunjucks views will get govukFrontend set to the internal GOV.UK Frontend version

Is there a router/middleware approach we can use to set:

  1. Set Nunjucks local govukFrontend to "internal version" by default
  2. Set Nunjucks local govukFrontend to "plugin version" in plugin pages only

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out Nunjucks, Sass and the error server had overlapping GOV.UK Frontend versions:

But now they're separated I've added separate locals for each:

// Add GOV.UK Frontend paths to Nunjucks locals
app.locals.govukFrontend = govukFrontend
app.locals.govukFrontendInternal = govukFrontendInternal

This allows prototype pages and management pages to clearly use their own versions:

{%- set assetPath = '/manage-prototype/dependencies/govuk-frontend' + govukFrontendInternal.assetPath -%}

Or for govuk-branded.njk it can be overridden or default to the plugin version:

{%- set assetPath = assetPath | default('/plugin-assets/govuk-frontend' + govukFrontend.assetPath) -%}

@colinrotherham colinrotherham force-pushed the frontend-v5-compatibility-colin branch 4 times, most recently from 60af033 to 5d73dd9 Compare September 13, 2023 16:00
@colinrotherham colinrotherham force-pushed the frontend-v5-compatibility-colin branch 7 times, most recently from 3ec7f0f to 88a76ec Compare September 14, 2023 16:16
Comment on lines +42 to +34
// GOV.UK Frontend plugin config
config: fse.readJsonSync(path.join(baseDir, 'govuk-prototype-kit.config.json'), {
throws: false
})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BenSurgisonGDS has suggested a new getScriptsAndAssetsConfig() utility for this

Couldn't find a "sync" way using an existing helper so might be a better option

@colinrotherham colinrotherham marked this pull request as ready for review September 14, 2023 16:24
@nataliecarey
Copy link
Contributor

Looks good to me, just needs a changelog entry.

@colinrotherham colinrotherham changed the title Automatically locate GOV.UK Frontend paths Locate GOV.UK Frontend using require.resolve() Sep 21, 2023
Replaces `getInternalGovukFrontendDir()` with `govukFrontendPaths()`

1. Prefers internal package in packageDir, then projectDir
2. Prefers plugin package in projectDir, then packageDir
Nunjucks locals have been added to prepare for a future GOV.UK Frontend v5 plugin update with a different directory structure
@colinrotherham colinrotherham merged commit c3f7655 into main Sep 21, 2023
27 checks passed
@colinrotherham colinrotherham deleted the frontend-v5-compatibility-colin branch September 21, 2023 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants