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

Upgrade to del and slash "ESM only" packages #2984

Merged
merged 4 commits into from
Nov 29, 2022
Merged

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Nov 8, 2022

Do we want to upgrade our pinned packages with "ESM only" updates?

There's a two-step process (unfortunately) which needs:

  1. Migrate Gulp and Node.js build tasks to ESM #2982
  2. Temporarily allow Babel to transpile "ESM only" dependencies for Jest

We need 2) until Node.js unflags --experimental-vm-modules for Jest ECMAScript Modules support

Update: Various related links

@colinrotherham colinrotherham requested a review from a team November 8, 2022 14:02
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2984 November 8, 2022 14:03 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2984 November 8, 2022 14:07 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2984 November 8, 2022 15:06 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2984 November 9, 2022 21:50 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2984 November 14, 2022 15:58 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2984 November 17, 2022 17:10 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2984 November 18, 2022 12:57 Inactive
Base automatically changed from gulp-esm to main November 21, 2022 17:56
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2984 November 21, 2022 17:59 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2984 November 28, 2022 13:21 Inactive
@colinrotherham
Copy link
Contributor Author

Going to make this ready for review as we've discussed it this morning

Likely due to a recent Jest or Babel package update (or the Node.js 18 upgrade) we no longer need to include child packages in transformIgnorePatterns which helps simplify the config changes

@colinrotherham colinrotherham changed the title For discussion: Upgrade "ESM only" packages Upgrade to del and slash "ESM only" packages Nov 28, 2022
@colinrotherham colinrotherham marked this pull request as ready for review November 28, 2022 13:26
@romaricpascal
Copy link
Member

Likely due to a recent Jest or Babel package update (or the Node.js 18 upgrade) we no longer need to include child packages in transformIgnorePatterns which helps simplify the config changes

Yeah, that makes things so much leaner than in the previous iteration you showed 🙌🏻 .

question Is there any reason we wouldn't want to configure Babel for all 4 projects? Worried that we one day add one of these packages to the projects and we miss that Babel is not set up for them.

suggestion: Is it worth having a ESM_ONLY_PACKAGES = ['del','slash'] constant we'd build the pattern from as a central point for listing packages we want Babel to process?

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2984 November 29, 2022 12:32 Inactive
@colinrotherham
Copy link
Contributor Author

Thanks @romaricpascal

question Is there any reason we wouldn't want to configure Babel for all 4 projects? Worried that we one day add one of these packages to the projects and we miss that Babel is not set up for them.

Don't mind at all. Assumed we'd only add it (temporarily) where needed—but pushed again 👍

suggestion: Is it worth having a ESM_ONLY_PACKAGES = ['del','slash'] constant we'd build the pattern from as a central point for listing packages we want Babel to process?

As the code reviewer, would you like that? 😊

Might not be necessary now it's only defined once at the top?

@colinrotherham colinrotherham merged commit 45c5f2f into main Nov 29, 2022
@colinrotherham colinrotherham deleted the esm-only-packages branch November 29, 2022 12:49
@colinrotherham colinrotherham added interoperability dependencies Pull requests that update a dependency file and removed tech debt labels Feb 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file interoperability tooling
Projects
Development

Successfully merging this pull request may close these issues.

3 participants