Migrate Gulp and Node.js build tasks to ESM #2982

colinrotherham · 2022-11-08T13:23:40Z

This PR upgrades our build tooling to ESM using *.mjs files

Why? We have a couple of packages noted in #2937 with "ESM only" upgrades

Intentionally ignored until we use ESM by default

del → ^7.0.0 – ESM export only, we need CommonJS
slash → ^5.0.0 – ESM export only, we need CommonJS

Both of them are flagged by dependabot:

Additional considerations

The following Gulp tasks have been removed by name but are still part of their npm script:

gulp scss:compile → npm run compile
gulp js:compile → npm run compile
gulp copy:assets → npm run build:dist
gulp copy:files → npm run build:package

For convenience I've kept gulp watch but perhaps npm start is enough?

I've also ensured new ESM imports have no "side effects" which means moving (some) gulp.task() calls to gulpfile.mjs which chips away a bit more at:

Improve and simplify Gulp #2717

colinrotherham · 2022-11-08T14:09:47Z

gulpfile.mjs

+ * Watch task
+ * Runs tasks when files are modified
+ */
+gulp.task('watch', watch)


I've added this so gulp watch still works, do we want this?

I say nah. Can't think of a good enough reason to use it outside of the dev task.

Thanks @domoscargin I'll remove it, rebase, and get this merged 😊

colinrotherham · 2022-11-08T14:11:04Z

gulpfile.mjs

-require('./tasks/gulp/compile-assets.js')
-require('./tasks/gulp/copy-to-destination.js')
-require('./tasks/gulp/watch.js')
+import { compileJavaScripts, compileStylesheets } from './tasks/gulp/compile-assets.mjs'


Are we happy losing the gulp js:compile and gulp scss:compile tasks?

They're still available via gulp compile which includes compileJavaScripts() and compileStylesheets()

I can see edge cases where somebody is working on some finicky interaction between CSS and JS, but that feels such an unlikely case, and these are not part of the "public" package json scripts, so I think we're OK to toss 'em.

colinrotherham · 2022-11-08T14:12:37Z

tasks/gulp/copy-to-destination.mjs

+ *
+ * @returns {import('stream').Stream} Output file stream
+ */
+export function copyAssets () {


Moved this one from gulpfile.mjs

Made sense to have copyAssets() (fonts, images) alongside copyFiles()

domoscargin

I don't have any blocking comments - I think this makes sense, especially since we're using ESM for most of our source code. Ran through the npm scripts and they seemed fine.

domoscargin · 2022-11-21T16:16:29Z

gulpfile.mjs

-require('./tasks/gulp/compile-assets.js')
-require('./tasks/gulp/copy-to-destination.js')
-require('./tasks/gulp/watch.js')
+import { compileJavaScripts, compileStylesheets } from './tasks/gulp/compile-assets.mjs'


I can see edge cases where somebody is working on some finicky interaction between CSS and JS, but that feels such an unlikely case, and these are not part of the "public" package json scripts, so I think we're OK to toss 'em.

domoscargin · 2022-11-21T16:37:21Z

gulpfile.mjs

+ * Watch task
+ * Runs tasks when files are modified
+ */
+gulp.task('watch', watch)


I say nah. Can't think of a good enough reason to use it outside of the dev task.

Also avoids the named `yargs` ESM export differing to the destructured `{ yargs }` CommonJS export

Avoids blocking a future upgrade to `slash@5` (ESM only) since we can’t yet use an ESM `postcss.config.mjs` without `postcss-load-config@4` We’ll need to keep a CommonJS config for a little longer

colinrotherham added tooling Frontend squad labels Nov 8, 2022

colinrotherham requested a review from a team November 8, 2022 13:23

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2982 November 8, 2022 13:23 Inactive

colinrotherham force-pushed the gulp-esm branch from eb2788f to fcfc213 Compare November 8, 2022 13:51

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2982 November 8, 2022 13:52 Inactive

domoscargin self-requested a review November 8, 2022 13:54

colinrotherham mentioned this pull request Nov 8, 2022

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

Merged

colinrotherham force-pushed the gulp-esm branch from fcfc213 to 0b031ee Compare November 8, 2022 14:07

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2982 November 8, 2022 14:07 Inactive

colinrotherham commented Nov 8, 2022

View reviewed changes

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2982 November 8, 2022 15:05 Inactive

colinrotherham force-pushed the gulp-esm branch from d7bf7cd to fb1de88 Compare November 9, 2022 21:37

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2982 November 9, 2022 21:37 Inactive

colinrotherham mentioned this pull request Nov 11, 2022

Bump slash from 3.0.0 to 5.0.0 #3003

Closed

colinrotherham force-pushed the gulp-esm branch from fb1de88 to c30ddb4 Compare November 14, 2022 15:56

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2982 November 14, 2022 15:56 Inactive

colinrotherham force-pushed the gulp-esm branch from c30ddb4 to 4d7cf6c Compare November 17, 2022 16:59

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2982 November 17, 2022 16:59 Inactive

colinrotherham force-pushed the gulp-esm branch from 4d7cf6c to 04638b8 Compare November 18, 2022 12:55

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2982 November 18, 2022 12:56 Inactive

colinrotherham added the tech debt label Nov 18, 2022

domoscargin approved these changes Nov 21, 2022

View reviewed changes

colinrotherham added 2 commits November 21, 2022 17:35

Prefer yargs-parser versus yargs CLI tool

2fbdeeb

Also avoids the named `yargs` ESM export differing to the destructured `{ yargs }` CommonJS export

Prefer minimatch to slash in PostCSS config

e72c9b0

Avoids blocking a future upgrade to `slash@5` (ESM only) since we can’t yet use an ESM `postcss.config.mjs` without `postcss-load-config@4` We’ll need to keep a CommonJS config for a little longer

colinrotherham force-pushed the gulp-esm branch from 04638b8 to b6f06dd Compare November 21, 2022 17:43

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2982 November 21, 2022 17:43 Inactive

Migrate Gulp and Node.js build tasks to ESM

39d7329

colinrotherham force-pushed the gulp-esm branch from b6f06dd to 39d7329 Compare November 21, 2022 17:51

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2982 November 21, 2022 17:51 Inactive

colinrotherham merged commit ec5fa30 into main Nov 21, 2022

colinrotherham deleted the gulp-esm branch November 21, 2022 17:56

colinrotherham added interoperability and removed tech debt labels Feb 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate Gulp and Node.js build tasks to ESM #2982

Migrate Gulp and Node.js build tasks to ESM #2982

colinrotherham commented Nov 8, 2022 •

edited

Loading

colinrotherham Nov 8, 2022

domoscargin Nov 21, 2022

colinrotherham Nov 21, 2022

colinrotherham Nov 8, 2022

domoscargin Nov 21, 2022

colinrotherham Nov 8, 2022

domoscargin left a comment

domoscargin Nov 21, 2022

domoscargin Nov 21, 2022

Migrate Gulp and Node.js build tasks to ESM #2982

Migrate Gulp and Node.js build tasks to ESM #2982

Conversation

colinrotherham commented Nov 8, 2022 • edited Loading

Additional considerations

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domoscargin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colinrotherham commented Nov 8, 2022 •

edited

Loading