chore: degulpify the package tasks by maribethb · Pull Request #10323 · RaspberryPiFoundation/blockly

maribethb · 2026-08-26T21:35:00Z

The basics

I validated my changes

The details

Resolves

Proposed Changes

Removes gulpfiles/package_tasks.js and replaces it with a function-by-function equivalent that uses plain node scripts instead of gulp tasks.
Removes gulpfiles/helper_tasks.js and replaces it with several different utility files with better-organized helper functions
Updates other gulpfiles that referenced those helpers to use the new functions instead
Noteworthy changes:
- Still uses gulp to run the build which is a prerequisite to packing, since the build is still a gulp task. At some point after build is no longer a gulp task, we should instead use the nx task graph to make sure that build is always run before packaging instead of relying on invoking it here, that way we get cacheing
- Removed one line that was calling .replace on the contents of all the msg files to "remove references to goog.module and goog.provide" -- there weren't any of those references present, and that is a bug waiting to happen if a translation ever contains the string goog
- we were using a gulp plugin called gulp-umd to do the umd wrapping, that's been replaced with a handwritten script that I think is not too difficult to maintain and hopefully in v14 we can stop doing umd altogether..
- uses promise.all to do some stuff in parallel that gulp was previously doing in parallel, using a guard to make sure we're not opening too many files at once

The biggest addition in this PR is scripts/package.mjs and if you review that side-by-side with the old (deleted in this PR) gulpfiles/package_tasks.js it's easier to understand since it really is function-by-function equivalent (with the exception above about the msg files)

Reason for Changes

This is faster and easier to read

Test Coverage

Verified that the results of dist/ are identical before and after this change.

Documentation

n/a

Additional Information

This change was created with assistance from Claude and manually verified / tested / rewritten where needed

maribethb · 2026-08-27T21:14:27Z

traced the failures to a goof that happened when i deleted the package-lock to deal with the npm bug. fixed in #10378

will rebase when that's merged and fix the package-lock again but correctly this time

gonfunko · 2026-09-03T19:57:43Z

+ */
+async function typings(buildFlags) {
+  await clean();
+  await runGulpTask('tsc', buildFlags);


The tsc gulp task looks to be two shell commands and takes no flags. Probably just inline it here and drop the buildFlags arg from this method?

gonfunko · 2026-09-03T20:05:17Z

AFAICT, this method is used only to invoke the tsc, cleanBuildDir, and build Gulp tasks. The first two are effectively one-liner shell commands, and the latter could be npm run build – maybe just do that and remove this file entirely?

gonfunko · 2026-09-03T20:07:45Z

This file feels like too much faffing about for one little rm – can it be inlined or consolidated into another file?

gonfunko · 2026-09-03T20:08:29Z

+ * The file is re-read on every call so that callers always see
+ * up-to-date data, even if it has been modified (e.g. to bump the
+ * version number) since the script started.


I'd just drop this, this is the behavior I'd expect by default.

Removed it, but expecting it by default caused problems for us previously and we had to do the weird cache-busting workaround because subsequent reads would read the cached value. But since this is a totally different way of reading the file, it's probably fine, it's just a footgun that we've hit before.

gonfunko · 2026-09-03T20:09:56Z

+/**
+ * A dependency to be injected into a UMD module.
+ *
+ * @typedef {object} UmdDependency
+ * @property {string} name Name of the dependency, used as the default
+ *     for any of the other properties that are omitted.
+ * @property {string=} amd Module ID to require in an AMD loader.
+ * @property {string=} cjs Module ID to require in a CJS loader.
+ * @property {string=} global Property of the global object (minus the
+ *     leading "root.") to use in a browser.
+ * @property {string=} param Name of the factory function parameter the
+ *     dependency is passed as.
+ */


Are these files actually typechecked, and if so can they just be Typescript?

not by the runtime environment, we'd have to compile the scripts before running them which seems like it would add needless complexity and time to the build. but most IDEs can handle reading the type declarations in comments so they're still useful when writing as your IDE can yell at you even if the running script won't

gonfunko · 2026-09-03T20:10:43Z

+ * Wrap the given script in a Universal Module Definition, so that it
+ * can be loaded by an AMD loader, by a CJS loader, or directly in a
+ * browser.
+ *
+ * This is a replacement for the gulp-umd plugin, supporting just the
+ * subset of its template syntax and options that our templates use.


"Wrap the given script in a Universal Module Definition." seems adequate here? The bit about it being a replacement I'd definitely drop.

I think it's useful to know this was yanked directly out of the gulp-umd plugin so that it can be updated or we can look there if we find a bug with this or whatever.

gonfunko · 2026-09-03T20:17:27Z

+}
+
+/**
+ * Write a text file, creating its parent directories if needed.


This doesn't seem to be limited to text files?

writeFile will assume it's utf8 encoding since we didn't specify otherwise, so I think it is. i.e. this would not work to write a binary file

gonfunko · 2026-09-03T20:19:02Z

+ *     (and so binary-safe) if it is omitted.
+ * @returns {Promise<void>} Promise resolved when copying is complete.
+ */
+export async function copyFiles({from, patterns, to, ignore = [], transform}) {


Can these just be regular args vs an options object?

it could, but I think the call sites are a lot more readable with the names since there's so many params. any particular reason you prefer regular args?

maribethb

Changed a few things:

moved cleanReleaseDir into fs_utils
created cleanBuildDir there too instead of using the gulp task for it, and then replaced the gulp task in build_tasks with this script version as well
used npm run build and npm run tsc instead of calling the underlying gulp build task directly
those two things let me remove the run gulp task script
added a run npm task script instead, the reason for that instead of just using spawnAsync directly is that windows can't spawn npm commands unless you set shell: true so this function handles that for you
dropped the build args from the typings command and updated the documentation to more clearly state they only apply to pack

maribethb · 2026-09-08T15:58:35Z

+}
+
+/**
+ * Write a text file, creating its parent directories if needed.


writeFile will assume it's utf8 encoding since we didn't specify otherwise, so I think it is. i.e. this would not work to write a binary file

maribethb · 2026-09-08T16:00:35Z

+ *     (and so binary-safe) if it is omitted.
+ * @returns {Promise<void>} Promise resolved when copying is complete.
+ */
+export async function copyFiles({from, patterns, to, ignore = [], transform}) {


it could, but I think the call sites are a lot more readable with the names since there's so many params. any particular reason you prefer regular args?

maribethb · 2026-09-08T16:04:36Z

+ * The file is re-read on every call so that callers always see
+ * up-to-date data, even if it has been modified (e.g. to bump the
+ * version number) since the script started.


Removed it, but expecting it by default caused problems for us previously and we had to do the weird cache-busting workaround because subsequent reads would read the cached value. But since this is a totally different way of reading the file, it's probably fine, it's just a footgun that we've hit before.

maribethb · 2026-09-08T16:11:20Z

+/**
+ * A dependency to be injected into a UMD module.
+ *
+ * @typedef {object} UmdDependency
+ * @property {string} name Name of the dependency, used as the default
+ *     for any of the other properties that are omitted.
+ * @property {string=} amd Module ID to require in an AMD loader.
+ * @property {string=} cjs Module ID to require in a CJS loader.
+ * @property {string=} global Property of the global object (minus the
+ *     leading "root.") to use in a browser.
+ * @property {string=} param Name of the factory function parameter the
+ *     dependency is passed as.
+ */


not by the runtime environment, we'd have to compile the scripts before running them which seems like it would add needless complexity and time to the build. but most IDEs can handle reading the type declarations in comments so they're still useful when writing as your IDE can yell at you even if the running script won't

maribethb · 2026-09-08T16:12:17Z

+ * Wrap the given script in a Universal Module Definition, so that it
+ * can be loaded by an AMD loader, by a CJS loader, or directly in a
+ * browser.
+ *
+ * This is a replacement for the gulp-umd plugin, supporting just the
+ * subset of its template syntax and options that our templates use.


I think it's useful to know this was yanked directly out of the gulp-umd plugin so that it can be updated or we can look there if we find a bug with this or whatever.

maribethb · 2026-09-08T19:46:39Z

Actually, now we don't need the npm run typings script at all since we switched to TypeDoc in the meantime and it doesn't use the generated dts files. So that simplified some things

maribethb requested a review from a team as a code owner August 26, 2026 21:35

maribethb requested a review from gonfunko August 26, 2026 21:35

github-actions Bot assigned gonfunko Aug 26, 2026

maribethb changed the title ~~Gulp package~~ chore: degulpify the package tasks Aug 26, 2026

github-actions Bot added the PR: chore General chores (dependencies, typos, etc) label Aug 26, 2026

maribethb added 4 commits August 28, 2026 12:50

chore: de-gulpify the package tasks

d45e2f5

chore: remove unnecessary sanitization

ad0dfbc

chore: format and fix package lock

e985f30

chore: actually fix package lock

0ea8014

maribethb force-pushed the gulp-package branch from a920f67 to 0ea8014 Compare August 28, 2026 18:03

maribethb added 2 commits August 28, 2026 14:11

Merge branch 'main' into gulp-package

5c28b8e

chore: pass compiler flags on from script

e3633ed

gonfunko reviewed Sep 3, 2026

View reviewed changes

chore: minor reorganization

a1b945d

maribethb commented Sep 8, 2026

View reviewed changes

maribethb added 2 commits September 8, 2026 15:06

Merge branch 'main' into gulp-package

d3c6e05

chore: remove now-unused typings script

c1ae218

Uh oh!

Conversation

maribethb commented Aug 26, 2026

The basics

The details

Resolves

Proposed Changes

Reason for Changes

Test Coverage

Documentation

Additional Information

Uh oh!

maribethb commented Aug 27, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maribethb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maribethb commented Sep 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants