Skip to content

Conversation

gribnoysup
Copy link
Collaborator

Last time we were doing major electron upgrade, we forgot to bump required node version to match. This is just a minor one this time, so not that big of a deal, but it's helpful to keep it in sync to make sure that the tests are running in the same env that the app is running

@gribnoysup gribnoysup requested a review from a team as a code owner September 18, 2025 07:31
@gribnoysup gribnoysup requested a review from mabaasit September 18, 2025 07:32
Copy link

Assigned lerouxb for team compass-developers because mabaasit is out of office.

@gribnoysup gribnoysup added the no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion) label Sep 18, 2025
@gribnoysup
Copy link
Collaborator Author

gribnoysup commented Sep 18, 2025

Seems like this minor bump reveals an issue somewhere in our units:

ReferenceError: require is not defined in ES module scope, you can use import instead

Moving to draft for now to take a closer look

@gribnoysup gribnoysup marked this pull request as draft September 18, 2025 08:17
Copy link
Collaborator

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

@gribnoysup I'd actually check whether this also appears with the latest Node.js 22.x version, this sounds like a familiar bug in Node.js's ESM compatibility that has actually been fixed in more recent versions

@gribnoysup
Copy link
Collaborator Author

Oh, interesting! I'll check, thanks for the pointer!

Copy link
Collaborator

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

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

Can you also update .tool-versions?

@gribnoysup
Copy link
Collaborator Author

Same on latest 22, not exactly sure yet what's going on, but seems like in these Node.js versions some ts files are not passed through the ts-node anymore when mocha is running, looking a bit deeper

@gribnoysup
Copy link
Collaborator Author

Oufff, okay, seems like in Node.js 22.15 what was happening is:

  1. mocha would try to load a .ts file as esm first
  2. this would fail with Unknown file extension
  3. mocha will fallback to require
  4. ts-node register for mocha will kick in, parse the file, everything is now commonjs there
  5. mocha is happy and running the test

In Node.js 22.18+ (maybe slightly earlier, I haven't tracked to exact version) .ts is now a known extension, so step 2 doesn't fail, Node.js will try to parse the file, ts-node/register doesn't kick in (because we haven't configured it for esm yet?), there's this mix of esm / require that Node.js detects, importing a file fails with unrecoverable error, mocha fails

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants