Migrate most grunt tasks off of grunt #5318

timmywil · 2023-09-12T03:33:44Z

Summary

Migrates most grunt tasks to npm scripts.

While there are a lot of changes, the only change to the built jQuery file is the version string. See below for full details.

I've tried to mostly keep the commits to one per task, but there are some early commits migrating the lint task in steps and some later commits to combine certain tasks (i.e. build, minify, dist, and compare_size) and simplify them. Here are some important things to notes:

Added a new build command using yargs to parse command line arguments and provide a pretty help command.
minify and dist always run with a build.
With grunt npmcopy removed, I committed the package-lock.json. We could actually commit a yarn.lock as well if we want. npm now updates both.
The new watch task takes advantage of rollup's watcher directly. I've removed the copying functionality from the dist task, which now only fulfills its original purpose of making sure output is ascii-only and doesn't contain carriage returns. I think the copying was added very early on to copy builds to a dropbox folder to test things on jsfiddle. Fortunately, if that's still needed, the build command-and therefore the watch task-can write to any directory and multiple can be run at once.
I've written a simplified version of compare_size that now lives in the jQuery repo and has no non-node dependencies other than chalk for pretty terminal colors. It uses Node's zlib directly instead of gzip-js or another npm package. gzip-js hasn't been touched in years. Fortunately, zlib is easy-to-use. Sizes are a bit larger than before using the default options, but that should be okay as we're looking for comparisons, not completely accurate size predictions.
I've moved the tests that don't require karma off of grunt. This includes the promises tests and node smoke tests.
I removed the necessary grunt tasks all together in the last commit
As I mentioned in the meeting, I've temporarily disabled an eslint-plugin-import rule (no-unused-imports) until the plugin supports the new flat config format.
the authors task, to replace grunt-git-authors, is already migrated in another PR: Release: migrate release process to release-it #5306

Version string

The version string now includes a short SHA when the build is not a release build (e.g. 4.0.0-pre+SHA). Also, I made the version follow semver syntax. So, the slim version would now be 4.0.0+slim. In a non-release build, that would look like 4.0.0-pre+slim.SHA. Custom builds still add excluded/included modules to the version string (when not the slim build), but I didn't bother adding those in a way that follows semver syntax (e.g. 4.0.0-pre+a85a6aa2 -ajax,-ajax/binary,etc). I'd like to remove -pre as well, but that change has already been made in my release PR (#5306).

Checklist

New tests have been added to show the fix or feature works
If needed, a docs issue/PR was created at https://github.com/jquery/api.jquery.com

TODO

~~I see that yargs throws an error on load in Node 10.x. I'll need to think on the best way to fix this.~~ Should be good now. Turns out concurrently has yargs as a dependency. yargs requires Node >=12, so it can't be used in the pretest script on Node 10.

- only run lint on latest node

test/server.js

mgol

Publishing what I have so far. Tomorrow, I'll resume with commit 386acf4 (#5318)

test/.eslintrc.cjs

test/node_smoke_tests/.eslintrc.cjs

.husky/commit-msg

.husky/pre-commit

package.json

build/tasks/lib/verifyNodeVersion.js

build/tasks/node_smoke_tests.js

test/node_smoke_tests/module/lib/jquery-module-specifier.js

Gruntfile.cjs

build/command.js

test/server.js

My remarks have been addressed

mgol

I reviewed the whole thing now. Nice work! I like a lot of the simplifications.

I haven't had a chance to test it locally, I'll do that next.

build/tasks/build.js

build/tasks/minify.js

build/tasks/dist.js

build/tasks/node_smoke_tests.js

build/tasks/build.js

timmywil · 2023-09-16T21:32:46Z

I think I've covered all the remaining feedback. However, how would you feel about changing the name of build-all-variants to something shorter, like build:all? I've added a separate commit to the end to see what it would look like.

See https://eslint.org/docs/latest/use/configure/configuration-files-new One rule currently blocked by import-js/eslint-plugin-import#2556

timmywil · 2023-09-16T22:45:46Z

I noticed an inconsistency between multiple compare_size runs and realized it's probably the version string varying by SHA. To fix it, compare_size now removes the banner for comparisons.

mgol · 2023-09-16T23:19:18Z

I think I've covered all the remaining feedback. However, how would you feel about changing the name of build-all-variants to something shorter, like build:all? I've added a separate commit to the end to see what it would look like.

Naming is hard. 🙂 I like build:all!

README.md

mgol · 2023-09-17T01:37:03Z

build/tasks/compare_size.mjs

+
+			let contents = await fs.promises.readFile( filename, "utf8" );
+
+			// Remove the banner for size comparisons.


It’s not just the banner, the jquery property varies as well.

However, I don’t think we should remove it. It’s what’s in the file and that’s what is getting served to the users; we should take that into account.

build/tasks/npmcopy.js

- confirmed they do not throw errors on Node 10 (still skipped)

- Output is a little wonky, but the tests are run in parallet to save time.

- this task was really a combination of two tasks, 1. process the files for distribution (ensure ascii and unix endings) 2. copy dist files somewhere else on the file system - should not be needed anymore, especially now with slim and module files, which it never supported.

- uses chalk, which requires esm modules

- move all the combinations to the JS file

timmywil · 2023-09-18T13:51:21Z

Remaining feedback addressed. As soon as we merge this, I'll start preparing the 3.x PR. I had already made a 3.x branch, but I need to copy the changes made from PR feedback.

mgol

I need to test this locally but let's merge, any feedback for how things work we can address separately.

timmywil added 3 commits September 11, 2023 21:25

Build: migrate lint tasks off grunt

9a477b0

Build: upgrade husky to 8.0.3; update lint command

8d5e631

Build: move build and lint to discrete action steps

3c772bd

- only run lint on latest node

github-advanced-security bot found potential problems Sep 12, 2023

View reviewed changes

test/server.js Fixed Show fixed Hide fixed

test/server.js Fixed Show fixed Hide fixed

timmywil force-pushed the grunt-migrate branch 4 times, most recently from 92eb540 to a85a6aa Compare September 12, 2023 16:11

mgol previously requested changes Sep 13, 2023

View reviewed changes

timmywil force-pushed the grunt-migrate branch 4 times, most recently from 802b344 to 11863e0 Compare September 14, 2023 02:46

github-advanced-security bot found potential problems Sep 14, 2023

View reviewed changes

test/server.js Fixed Show resolved Hide resolved

timmywil force-pushed the grunt-migrate branch 5 times, most recently from 8ffaa85 to 6d0e2f2 Compare September 15, 2023 03:22

mgol reviewed Sep 16, 2023

View reviewed changes

timmywil force-pushed the grunt-migrate branch from 6d0e2f2 to a3b2403 Compare September 16, 2023 21:27

Build: migrate eslint config to flat config

094d9d4

See https://eslint.org/docs/latest/use/configure/configuration-files-new One rule currently blocked by import-js/eslint-plugin-import#2556

timmywil force-pushed the grunt-migrate branch 4 times, most recently from 9576492 to 8c8e14f Compare September 16, 2023 22:44

mgol reviewed Sep 17, 2023

View reviewed changes

timmywil added 16 commits September 18, 2023 09:40

Tests: migrate npmcopy off of grunt; commit package-lock.json

a9a0701

Build: move pretest scripts out of grunt

cf9d1a5

Tests: add express test server that uses middleware-mockserver

bb6977c

Build: migrate node smoke tests off grunt

46ba2fb

- confirmed they do not throw errors on Node 10 (still skipped)

Build: move promises aplus tests out of grunt

f7165dc

- Output is a little wonky, but the tests are run in parallet to save time.

Build: migrate grunt build off of grunt; add yargs

8d5fcf7

Build: migrate grunt minify task

e9f4a24

Build: migrate grunt compare size task off of grunt

1b71a0d

- uses chalk, which requires esm modules

Build: automatically run dist/compare_size in minifyDefaultFiles

61af944

Build: add watch task to build, minify, and run dist

de7d5cd

Build: document build task; simplify commands

d274e18

Build: remove stale grunt tasks

a38b5bd

Build: add necessary build scripts to test scripts

8f47330

Build: consolidate node_smoke_tests tasks into one

9fb93f9

- move all the combinations to the JS file

Build: rename build-all-variants script to build:all

76c85ac

timmywil force-pushed the grunt-migrate branch from 8c8e14f to 76c85ac Compare September 18, 2023 13:50

mgol approved these changes Sep 18, 2023

View reviewed changes

timmywil merged commit 2bdecf8 into jquery:main Sep 18, 2023

mgol mentioned this pull request Sep 18, 2023

New build system: re-add commit hashes in compare_size #5327

Closed

mgol added this to the 4.0.0 milestone Sep 19, 2023

mgol added the Build label Sep 19, 2023

mgol mentioned this pull request Sep 20, 2023

Build: migrate most grunt tasks off of grunt (3.x) #5330

Merged

2 tasks

timmywil deleted the grunt-migrate branch December 28, 2023 15:47

mgol mentioned this pull request Jan 29, 2024

Tests: Fix Karma tests on Node.js 20 #5397

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate most grunt tasks off of grunt #5318

Migrate most grunt tasks off of grunt #5318

timmywil commented Sep 12, 2023 •

edited

Loading

mgol left a comment

mgol left a comment

timmywil commented Sep 16, 2023 •

edited

Loading

timmywil commented Sep 16, 2023

mgol commented Sep 16, 2023

mgol Sep 17, 2023

timmywil commented Sep 18, 2023 •

edited

Loading

mgol left a comment


		let contents = await fs.promises.readFile( filename, "utf8" );

		// Remove the banner for size comparisons.

Migrate most grunt tasks off of grunt #5318

Migrate most grunt tasks off of grunt #5318

Conversation

timmywil commented Sep 12, 2023 • edited Loading

Summary

Checklist

TODO

mgol left a comment

Choose a reason for hiding this comment

mgol left a comment

Choose a reason for hiding this comment

timmywil commented Sep 16, 2023 • edited Loading

timmywil commented Sep 16, 2023

mgol commented Sep 16, 2023

mgol Sep 17, 2023

Choose a reason for hiding this comment

timmywil commented Sep 18, 2023 • edited Loading

mgol left a comment

Choose a reason for hiding this comment

timmywil commented Sep 12, 2023 •

edited

Loading

timmywil commented Sep 16, 2023 •

edited

Loading

timmywil commented Sep 18, 2023 •

edited

Loading