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.

Sign up for GitHub

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

Jump to bottom

Fix package resolution in Node.js 17+ for support/4.x #3817

Merged
merged 4 commits into from
Jun 22, 2023
Merged

Fix package resolution in Node.js 17+ for support/4.x #3817

merged 4 commits into from
Jun 22, 2023

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Jun 16, 2023 •
edited
Loading

As described in #3755 this PR fixes point 1)

We've identified two scenarios when trying to require() or import GOV.UK Frontend

  1. Node.js >= 17 errors with GOV.UK Frontend v4 folder mapping in "exports"
  2. Node.js <= 12.18 errors with GOV.UK Frontend v5 (unreleased) subpath patterns in "exports"

This may also fix issues with bundlers that also follow Node.js package exports resolution

@colinrotherham
Group package fields together
807c921 
For context Sass does not resolve package.json fields or exports
sass/sass#2739

But bundlers like webpack use “style” and “sass” fields using loaders:

https://www.npmjs.com/package/css-loader
https://www.npmjs.com/package/sass-loader

Note: With “exports” present in package.json the stylesheet exports must appear there too to maintain both backwards and forwards compatibility
@colinrotherham
Fix package.json exports for Node.js >= 17
942fe6f 
We hadn’t realised that support for “subpath exports” with trailing slashes (not asterisks) was deprecated in Node.js 16 and removed in Node.js 17 https://nodejs.org/docs/latest-v18.x/api/deprecations.html#dep0148-folder-mappings-in-exports-trailing-

But fully switching to “subpath patterns” requires Node.js 12.20+ so both flavours will be needed for backwards compatibility

Fixes: #3755
@colinrotherham colinrotherham added 🐛 bug Something isn't working the way it should (including incorrect wording in documentation) javascript labels Jun 16, 2023
@colinrotherham colinrotherham requested a review from a team as a code owner June 16, 2023 13:17
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3817 June 16, 2023 13:17 Inactive
@colinrotherham colinrotherham changed the title Fix Node.js >= 17 require + import (for v4.6.x) Fix package resolution in Node.js 17+ Jun 16, 2023
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3817 June 16, 2023 13:24 Inactive
@colinrotherham colinrotherham changed the title Fix package resolution in Node.js 17+ Fix package resolution in Node.js 17+ in support/4.6.x Jun 16, 2023
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3817 June 16, 2023 13:32 Inactive
This was referenced Jun 16, 2023
Closed
Closed
@colinrotherham colinrotherham changed the title Fix package resolution in Node.js 17+ in support/4.6.x Fix package resolution in Node.js 17+ for support/4.6.x Jun 16, 2023
@36degrees
Copy link
Contributor

Does this need changing to target support/4.x?

@colinrotherham
Copy link
Contributor Author

Does this need changing to target support/4.x?

I've held off doing it to save time but if we're 100% sure then I'll proceed 🚀

@colinrotherham colinrotherham changed the base branch from support/4.6.x to support/4.x June 19, 2023 16:03
@colinrotherham colinrotherham changed the title Fix package resolution in Node.js 17+ for support/4.6.x Fix package resolution in Node.js 17+ for support/4.x Jun 19, 2023
@colinrotherham
Copy link
Contributor Author

colinrotherham commented Jun 19, 2023 •
edited
Loading

Target branch changed

@colinrotherham colinrotherham added this to the v4.7.0 milestone Jun 21, 2023
romaricpascal
romaricpascal approved these changes Jun 22, 2023
View reviewed changes
Copy link
Member

@romaricpascal romaricpascal left a comment

Choose a reason for hiding this comment

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

Good to have the opportunity to release that one in 4.7.0 🥳

@colinrotherham colinrotherham merged commit b68f58b into support/4.x Jun 22, 2023
@colinrotherham colinrotherham deleted the node-require-v4 branch June 22, 2023 13:18
@beclamide beclamide mentioned this pull request Aug 19, 2024
Closed
@beclamide beclamide mentioned this pull request Sep 2, 2024
Open
@AbidemiAdio AbidemiAdio mentioned this pull request Sep 19, 2024
Closed
@GodsonLeigh GodsonLeigh mentioned this pull request Sep 19, 2024
Open
@ajaysoni-cap ajaysoni-cap mentioned this pull request Sep 19, 2024
Closed
This was referenced Sep 19, 2024
Open
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romaricpascal romaricpascal romaricpascal approved these changes

Assignees
No one assigned
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation) javascript
Projects
GOV.UK Design System cycle board
Archived in project
Milestone
v4.7.0
Development

Successfully merging this pull request may close these issues.
4 participants
@colinrotherham @36degrees @romaricpascal @govuk-design-system-ci