:zombie: v3.0.2 by moonglum · Pull Request #233 · faucet-pipeline/faucet-pipeline-js

moonglum · 2025-01-26T10:04:19Z

Re-energized by Node finally supporting real compatibility between ESM and CommonJS, I wanted to see how much work it is to update everything to the latest versions. Doesn't seem to be a lot:

I adjusted the test output, as browser support for newer features has gotten better (who knew!)
Then I removed mocha, as we can use the built-in test runner with no code changes
Updated the dependencies to the latest versions with one exception (all without any code changes)
- The updates also remove the deprecation warnings you see in the version of faucet-js that is currently released
Updated the node support matrix (still supporting v18 in this release to make it non-breaking. Furthermore, it is still supported until April and there is no reason not to do it)

The one dependency I did not update to the latest version is @rollup/plugin-commonjs. Version 27 changes the output in a weird way.

Then I cherry-picked two more changes from the swc branch:

Drop support for undocumented module format names (es and es6)
Bugfix: NAMELESS_MODULES used es instead of esm

FND

Looks pretty good; excellent work, thank you!

I've only looked at the diff/commits so far, i.e. haven't actually executed any code yet - but will have to do all that anyway as part of the release process (which always scared me a little, even back when I was still familiar with those shenanigans... ).

FND · 2025-01-26T11:46:17Z

lib/bundle/config.js

 let MODULE_FORMATS = { // maps faucet identifiers to Rollup identifiers
 	esm: true,
-	es: "esm", // alias
-	es6: "esm", // alias


I honestly have no idea why we had those aliases in the first place; "esm" probably wasn't firmly established back then? 👴

Yes, exactly

FND · 2025-01-26T11:46:37Z

lib/bundle/config.js

 	iife: true
 };
-let NAMELESS_MODULES = new Set(["es", "amd", "cjs"]); // NB: Rollup identifiers
+let NAMELESS_MODULES = new Set(["esm", "amd", "cjs"]); // NB: Rollup identifiers


Just to be sure: This should always have been "esm" as far as Rollup (not faucet) was concerned?

This is my understanding, yes!

FND · 2025-01-26T11:47:49Z

package.json

 		"test": "npm-run-all lint --parallel test:unit test:cli",
 		"test:cli": "./test/cli/run",
-		"test:unit": "mocha test/unit/test_*.js",
+		"test:unit": "node --test test/unit/test_*.js",


This is pretty cool - thank you for caring!

Just to be safe: Does this actually execute the same amount of tests? (I'm afraid I can't check myself at this time.)

Suggested change

"test:unit": "node --test test/unit/test_*.js",

"test:unit": "node --test ./test/unit/test_*.js",

Yes, it executes all tests. I also made a test fail to make sure the exit code is correct. Will fixup this change 👍

test/unit/test_bundling.js

moonglum · 2025-01-26T13:39:59Z

@FND Adressed all your feedback, and force pushed the change 👍 Let me know if anything is missing.

FND

LGTM

test/unit/test_bundling.js

it appears this was never correct in the first place

@moonglum

as suggested by @moonglum: this appears to be unmaintained, but it's not really worth the encumbrance anyway

FND · 2025-02-01T10:57:28Z

@moonglum: I've added a couple of commits in preparation for the release. Note that I opted for v3.0.1 instead of v3.1, though I'm not entirely sure about that...

560c75c removed npm-run-all dependency
7780cf2 v3.0.2

Please note the newly added change log (very similar to faucet-pipeline/faucet-pipeline-core#147).

Failing tests are expected because we're referencing pre-release versions (due to the weirdness of our DIY monorepo here).

I've also allowed myself to rewrite history while I was reviewing commits for the change log, mostly to adjust commit messages' grammar for consistency.

moonglum · 2025-02-01T12:25:59Z

Thanks for getting this ready for release 👍 No objections to making this a 3.0.2 release instead of a 3.1 release. npm-run-all is definitely not a reason against it, as it is a dev dependency. So why would anyone depending on this package care? The only reason would be dropping support for Node 18. But I'm okay with that.

FND · 2025-02-02T12:23:09Z

The only reason would be dropping support for Node 18.

Indeed, but that would be a major rather than a minor version change then? So yeah, 3.0.2 it is.

moonglum · 2025-02-02T21:35:18Z

Great, then I think this one is ready for release 👍

note that we don't actually _need_ faucet-core v3 though because `await` doesn't care whether there's a promise or not

FND · 2025-03-02T20:28:17Z

@moonglum: updated to account for faucet-core v3 see c880cac - though we'll have to update the change log after deciding on a version number

moonglum · 2025-03-04T21:01:13Z

Thanks! I would probably still set to core v3 only. We need to make sure that all pipelines use the same version of faucet, otherwise things break. I would therefore set them all to require core 3.

Update: Let's discuss that aspect in the core PR.

moonglum · 2025-03-08T07:57:35Z

CHANGELOG.md

 ==================================


+v3.0.2


I think we will go for 3.1.0, right?

moonglum requested a review from FND January 26, 2025 10:04

FND approved these changes Jan 26, 2025

View reviewed changes

moonglum force-pushed the 3.1 branch from 32ef114 to e62fa37 Compare January 26, 2025 13:38

FND approved these changes Jan 27, 2025

View reviewed changes

test/unit/test_bundling.js Show resolved Hide resolved

moonglum added 2 commits February 1, 2025 11:25

updated expected transpilation results

7126906

replaced Mocha with Node's built-in test runner

f3a856c

FND force-pushed the 3.1 branch from e62fa37 to 01cac75 Compare February 1, 2025 10:36

moonglum and others added 5 commits February 1, 2025 11:42

updated dependencies

01d28ba

dropped support for obsolete versions of Node

071f3f2

dropped support for undocumented module-format aliases

c83c333

fixed nameless module designations

85fefeb

it appears this was never correct in the first place

removed npm-run-all dependency

560c75c

as suggested by @moonglum: this appears to be unmaintained, but it's not really worth the encumbrance anyway

FND force-pushed the 3.1 branch 2 times, most recently from 54d2013 to 7780cf2 Compare February 1, 2025 10:50

FND changed the title ~~🧟 v3.1~~ 🧟 v3.0.1 Feb 1, 2025

FND changed the title ~~🧟 v3.0.1~~ 🧟 v3.0.2 Feb 2, 2025

FND added 2 commits March 2, 2025 21:25

updated internals for compatibility with faucet-core v3

c880cac

note that we don't actually _need_ faucet-core v3 though because `await` doesn't care whether there's a promise or not

v3.0.2

f99b958

FND force-pushed the 3.1 branch from 7780cf2 to f99b958 Compare March 2, 2025 20:26

FND mentioned this pull request Mar 2, 2025

v3.0.0 faucet-pipeline/faucet-pipeline-core#147

Merged

moonglum commented Mar 8, 2025

View reviewed changes

CHANGELOG.md

==================================

v3.0.2

Copy link

Member Author

moonglum Mar 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we will go for 3.1.0, right?

moonglum mentioned this pull request Mar 8, 2025

faucet-pipeline-core 4.0.0 rollout faucet-pipeline/faucet-pipeline-core#149

Open

6 tasks

	"test:unit": "node --test test/unit/test_*.js",
	"test:unit": "node --test ./test/unit/test_*.js",

Conversation

moonglum commented Jan 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FND left a comment

Choose a reason for hiding this comment

Uh oh!

FND Jan 26, 2025

Choose a reason for hiding this comment

Uh oh!

moonglum Jan 26, 2025

Choose a reason for hiding this comment

Uh oh!

FND Jan 26, 2025

Choose a reason for hiding this comment

Uh oh!

moonglum Jan 26, 2025

Choose a reason for hiding this comment

Uh oh!

FND Jan 26, 2025 • 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!

moonglum Jan 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

moonglum commented Jan 26, 2025

Uh oh!

FND left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

FND commented Feb 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

moonglum commented Feb 1, 2025

Uh oh!

FND commented Feb 2, 2025

Uh oh!

moonglum commented Feb 2, 2025

Uh oh!

FND commented Mar 2, 2025

Uh oh!

moonglum commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

moonglum Mar 8, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

moonglum commented Jan 26, 2025 •

edited

Loading

FND Jan 26, 2025 •

edited

Loading

FND commented Feb 1, 2025 •

edited

Loading

moonglum commented Mar 4, 2025 •

edited

Loading