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

Allow users to use require.resolve to import GOV.UK Frontend JavaScript #2644

Merged
merged 2 commits into from
Jun 7, 2022
Merged

Allow users to use require.resolve to import GOV.UK Frontend JavaScript #2644

merged 2 commits into from
Jun 7, 2022

Conversation

vanitabarrett
Copy link
Contributor

@vanitabarrett vanitabarrett commented May 25, 2022 •
edited
Loading

Fixes #2643

What

Add require entry point to package/package.json file.

Why

When we introduced ECMAScript modules, we broke the ability to require the
JavaScript. By defining an entry point for require statements in the package.json
file, we add back that functionality.

https://nodejs.org/api/packages.html#conditional-exports

Tested with

  • Basic require setup
  • Webpack, to ensure ESM import not affected
  • Rollup, to ensure ESM import not affected
  • Parcel, to ensure ESM import not affected
  • esbuild, to ensure ESM import not affected
  • Prototype Kit, to ensure change is still backwards compatible
  • Design System

Allow users to require GOV.UK Frontend JavaScript
0b372f9 
When we introduced ECMAScript modules, we broke the ability to `require` the
JavaScript. By defining an entry point for `require` statements in the package.json
file, we add back that functionality.

https://nodejs.org/api/packages.html#conditional-exports
@vanitabarrett vanitabarrett changed the title Allow users to require GOV.UK Frontend JavaScript Allow users to use require.resolve to import GOV.UK Frontend JavaScript May 25, 2022
@vanitabarrett vanitabarrett marked this pull request as ready for review May 25, 2022 11:30
@vanitabarrett vanitabarrett requested a review from a team May 25, 2022 11:31
domoscargin
domoscargin approved these changes May 31, 2022
View reviewed changes
Copy link
Contributor

@domoscargin domoscargin left a comment

Choose a reason for hiding this comment

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

This looks good to me, and I can replicate both the failing and succeeding behaviour.

@vanitabarrett vanitabarrett merged commit 7dc1050 into main Jun 7, 2022
@vanitabarrett vanitabarrett deleted the require-js-fix branch June 7, 2022 09:46
@domoscargin domoscargin mentioned this pull request Jun 27, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@domoscargin domoscargin domoscargin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4.1.0 breaks require.resolve to find all.js client javascript
2 participants
@vanitabarrett @domoscargin