chore: pull build and test-compile-advanced task out of gulp by lizschwab · Pull Request #10546 · RaspberryPiFoundation/blockly

lizschwab · 2026-09-29T23:06:53Z

The basics

I validated my changes

The details

Resolves

Moves the build and test-compile-advanced tasks off of gulp and into node-based scripts.

Proposed Changes

Remove build, minify, and buildAdvancedCompilationTest gulp tasks
Remove minify script, which was made redundant when we restructured the build task and moved tsc and langfiles scripts onto the npm commands (see chore: move tsc command off gulp #10519 and chore: move langfiles and messages off gulp #10522)
Delete scripts/gulpfiles/build_tasks.mjs
Add new node-based scripts, scripts/build.mjs and tests/compile/test_compile_advanced.mjs
Noteable change: we are no longer stripping licenses on the test compile; this is not necessary
Note that the results of the test_compile_advanced.mjs do differ from the original output; this is due to differences in how the different versions of the closure compiler differing in how they build out their lists when using the ADVANCED_OPTIMIZATIONS. As this is only a test and the test is passing as expected, we don't need to be concerned about this change.
A couple of nx configurations were cleaned up.

Reason for Changes

Removing dependency on gulp

Test Coverage

I validated these changes by doing the following:

Running a diff between the contents of the /dist folder after gulp version of the build command and after the new node command
Running a plugin locally against the compressed files to ensure the sourcemaps work correctly (to test this yourself, you can update the webpack.config.js from devtool: isProduction ? 'source-map' : 'eval-source-map', to devtool: 'source-map')
Running through various permutations of npm ci, npm run build, npm run test, npm run lint, and npm run format in addition to running each test script individually and running each changed script individually to ensure they work consistently and as expected.

Additional Information

AI was used to build out the function that calls the google-closure-compiler to make the output identical to the old gulp output to give us high confidence in the output. It was also used to help debug some issues encountered during testing. All changes were verified by a human and thoroughly vetted against the gulp version of the output of these tasks.

maribethb · 2026-10-06T18:16:09Z

+      const source = await readFile(filePath, 'utf8');
+      // Strip out Google's and MIT's Apache licences.
+      // Closure Compiler preserves dozens of Apache licences in the Blockly code.
+      // Remove these if they belong to Google or MIT.


Or RPF (I think the regex was already updated to do this, but you can doublecheck)

maribethb · 2026-10-06T18:17:09Z

  'visibility',
 ];

+var languages = null;


this should be a let, no reason to use var anymore, right?

maribethb · 2026-10-06T18:26:51Z

-export const buildAdvancedCompilationTest = gulp.series(
-  compileAdvancedCompilationTest,
-);
+build();


test_compile_advanced imports the compile function from this file, and because of this line it would then kick off a build, but the adv compile test already does its own build too which is going to be running at the same time this one is. and since this modifies files on disk that is going to be problematic.

the easiest solution is probably moving compile and anything else that's imported from a different file to its own module with no side effects, and then this line needs a top-level await so that any following scripts actually wait for the build to complete

maribethb · 2026-10-06T18:28:01Z

+ * This will normalize the command across platforms (e.g. python3 on Linux and
+ * Mac, python on Windows).
+ */
+const PYTHON = process.platform === 'win32' ? 'python' : 'python3';


this is already in build_constants, we should just import it from there.

maribethb · 2026-10-06T18:45:47Z

+  );
+}
+
+compileAdvancedCompilationTest();


i think this might need an await, to make sure it finishes before the next thing happens. same for build. it would match the pattern we have in clean.mjs

also newline at EOF

maribethb · 2026-10-06T18:47:07Z

-    "build-strict": "gulp build --verbose --strict",
-    "build-strict-log": "npm run build:strict > build-debug.log 2>&1 && tail -3 build-debug.log",
+    "build": "npm run tsc && node \"scripts/build.mjs\" && npm run langfiles",
+    "build-debug": "npm run build --verbose --debug",


I think these flags are being passed to npm now, not all the way through to the build script. and you can't just add another layer of -- because then they'd be passed to npm run langfiles since that's the last command in the npm run build sequence. so I think this needs to be rethought a little bit. same for below.

though npm run langfiles in the build command seems redundant since you're already running that as part of the build script

maribethb · 2026-10-06T18:49:39Z

this one can probably be removed now, right. and maybe some of the others

maribethb · 2026-10-06T19:14:57Z

    "lint": "eslint .",
    "lint-fix": "eslint . --fix",
    "langfiles": "node \"scripts/langfiles.mjs\"",
-    "minify": "gulp minify",


the nx file still references this

maribethb

also i ran prepareDemos locally and the _deploy isn't gitignored so it creates noise. need to add packages/blockly/_deploy to the gitignore

you can run this locally to check to make sure the paths are all correct with:

npm run prepareDemos
npx http-server _deploy -p 8081

and open http://localhost:8081/static/packages/blockly/tests/playgrounds/advanced_playground.html?compressed=true

maribethb · 2026-10-15T19:55:35Z

+    create_source_map: mapPath,
+  };
+
+  await compile(options);


So we're passing the --debug flag when we actually call this script, but I don't see anywhere you're parsing it. I think you need to use yargs and forward argv onto compile(options, argv) which is what the build script does. otherwise adv compilation test will no longer run with the extra debug args.

maribethb · 2026-10-15T19:58:26Z

is this the right path still?

maribethb · 2026-10-15T20:00:33Z

+/**
+ * Build all chunks, compressed files, sourcemaps, shims, and langfiles.
+ *
+ * Prerequisite: tsc (handled by nx).


this says tsc is a prereq handled by nx, but the npm script starts with npm run tsc && so it seems like we're handling it manually, maybe? also for the adv compilation test

maribethb · 2026-10-15T21:16:40Z

+      '../../node_modules/@blockly/theme-modern/dist/index.js',
+      '../../node_modules/@blockly/block-test/dist/index.js',
  ];
  return gulp.src(playgroundDeps, {base: '.'}).pipe(gulp.dest(demoStaticTmpDir));


I think this needs to be:

return gulp.src(playgroundDeps, {base: '../..'}).pipe(gulp.dest(`${demoStaticTmpDir}/packages/blockly`));

running the current version puts these files in e.g. packages/blockly/node_modules/@blockly/dev-tools/dist/index.js where the advanced playground is looking for them in _deploy/static/packages/blockly/node_modules/@blockly/dev-tools/dist/index.js

maribethb · 2026-10-15T21:26:37Z

this needs a similar fix as to the playground deps:

return gulp.src(['build/msg/*', 'dist/*_compressed.js*', 'build/*.loader.mjs'], {base: '.'}) .pipe(gulp.dest(`${demoStaticTmpDir}/packages/blockly`));

maribethb · 2026-10-16T20:28:34Z

 function copyCompressedToOldLocation(done) {
  return gulp.src(['dist/*_compressed.js*'])
-      .pipe(gulp.dest(demoStaticTmpDir));
+      .pipe(gulp.dest(demoStaticBlocklyDir));


this and the one below it don't need to change, this function was explicitly copying the files to the top level demo static directory, for legacy reasons. these files are already copied to domeStaticBlocklyDir by copyBuilt so this would be redundant. everything else lgtm!

chore: pull build and test-compile-advanced task out of gulp

5ccf70b

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

lizschwab added 2 commits September 30, 2026 09:57

chore: resolve build shim file differences

16c8e48

Merge remote-tracking branch 'origin/main' into degulpify-build-task

14e58e0

lizschwab marked this pull request as ready for review September 30, 2026 19:28

lizschwab requested a review from a team as a code owner September 30, 2026 19:28

lizschwab requested a review from mikeharv September 30, 2026 19:28

github-actions Bot assigned mikeharv Sep 30, 2026

lizschwab requested review from maribethb and removed request for mikeharv October 2, 2026 21:09

github-actions Bot assigned maribethb Oct 2, 2026

maribethb reviewed Oct 6, 2026

View reviewed changes

lizschwab unassigned mikeharv Oct 13, 2026

lizschwab added 7 commits October 13, 2026 12:36

chore: small code quality fixes

8992464

chore: remove langfile redundancies

6224651

chore: move shared function, stop altering sourcemap for parity

ee55cf8

chore: replace glob-stream with globSync

27db464

chore: update appengine gulp command to use shell for build

841e54e

chore: update outdated file paths in appengine script

7f8dbfd

chore: remove unused gulp deps

a5be679

lizschwab requested a review from maribethb October 14, 2026 19:28

lizschwab added 4 commits October 14, 2026 13:37

Merge branch 'main' into degulpify-build-task

6c2b1c6

chore: linting/formatting

7d00797

chore: formatting

9bcb315

chore: fixed nx dep issue

04469f0

maribethb reviewed Oct 15, 2026

View reviewed changes

lizschwab added 2 commits October 16, 2026 09:46

chore: update appengine paths

deabc14

chore: address feedback

c6a1947

lizschwab requested a review from maribethb October 16, 2026 18:01

lizschwab added 2 commits October 16, 2026 11:02

chore: remove duplicated line

373d648

chore: fix test imports

d78b6df

maribethb approved these changes Oct 16, 2026

View reviewed changes

chore: reverted paths in old location functions

11eb13a

lizschwab merged commit ce8839d into RaspberryPiFoundation:main Oct 16, 2026
6 checks passed

Conversation

lizschwab commented Sep 29, 2026

The basics

The details

Resolves

Proposed Changes

Reason for Changes

Test Coverage

Additional Information

Uh oh!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maribethb left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

maribethb left a comment •

edited

Loading