Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking changes in 18.19.0 (including The --loader flag was deprecated in Node v20.6.0) #51048

Closed
everett1992 opened this issue Dec 4, 2023 · 8 comments

Comments

@everett1992
Copy link
Contributor

Version

18.19.0

Platform

Linux ua0683ea36b1857.ant.amazon.com 5.15.0-91-generic #101~20.04.1-Ubuntu SMP Thu Nov 16 14:22:28 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

various

What steps will reproduce the bug?

I'm seeing various failures in our CI builds after upgrading 18.18.0 to 18.19.0, including an error for a flag that is deprecated in node 20.6.0

(tsx 3) errors on flag deprecated in 20

$ node -v
v18.19.0
$ npx tsx@3 index.ts

node:internal/process/esm_loader:40
      internalBinding('errors').triggerUncaughtException(
                                ^
Error: tsx must be loaded with --import instead of --loader
The --loader flag was deprecated in Node v20.6.0
    at X (file:///home/ANT.AMAZON.COM/calebev/tmp/node_modules/tsx/dist/esm/index.mjs:1:1920)
    at Hooks.addCustomLoader (node:internal/modules/esm/hooks:202:24)
    at Hooks.register (node:internal/modules/esm/hooks:168:16)
    at async initializeHooks (node:internal/modules/esm/utils:167:5)
    at async customizedModuleWorker (node:internal/modules/esm/worker:104:24)

Node.js v18.19.0

(jest 28) TypeError: Cannot assign to read only property 'performance' of object '[object global]'

$ node -v
v18.19.0
$  echo "jest.useFakeTimers()" > jest.test.js
$ npx jest@28 jest.test.js
 FAIL  ./jest.test.js
  ● Test suite failed to run

    TypeError: Cannot assign to read only property 'performance' of object '[object global]'

    > 1 | jest.useFakeTimers()
        |                     ^
      2 |

      at hijackMethod (node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:946:32)
      at Object.install (node_modules/@sinonjs/fake-timers/src/fake-timers-src.js:1733:17)
      at FakeTimers.useFakeTimers (node_modules/@jest/fake-timers/build/modernFakeTimers.js:110:36)
      at Object.<anonymous> (jest.test.js:1:21)

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior? Why is that the expected behavior?

No breaking changes between minor versions of an LTS release

What do you see instead?

Breaking changes, including a reference to a flag that is deprecated in a greater major version.

Additional information

No response

@connor4312
Copy link
Contributor

In Node 18, the loader flag was marked as experimental for which they say:

The feature is not subject to semantic versioning rules. Non-backward compatible changes or removal may occur in any future release. Use of the feature is not recommended in production environments.

@targos
Copy link
Member

targos commented Dec 7, 2023

The error seems to be thrown by your dependency tsx. --loader still works in v18.19.0.

@everett1992
Copy link
Contributor Author

I'll try to create simple reproductions without dependencies. We have a few hundred packages failing to build with 18.19.0 that are building successfully with 18.18.0. Most are fixed by updating their dependencies to versions that work with node 20.

I'll accept that loader is experimental, but the error message should at least read deprecated in 18.19.0 instead of 20.6.0.

@targos
Copy link
Member

targos commented Dec 7, 2023

The error message does not come from us. It is thrown by the tsx package.

@aduh95
Copy link
Contributor

aduh95 commented Dec 20, 2023

Breaking existing code is painful, but it's a tradeoff to be made between having two different implementations between 18.x and the other release lines – which is a maintenance burden, both for us and for loaders/hooks authors. It's unfortunate that the package you are using is hiding the fact that it's using an experimental API (by default, Node.js emits a warning when you use the --loader flag), and in hindsight the decision to put a hardcoded version in the warning was probably a mistake, so you should report that to them (but IIRC major loader implementors (incl tsx) have been involved in the backport process and some have released patched versions even before Node.js v18.19.0 was out, so maybe try upgrading your deps first).

Breaking changes in experimental APIs is considered fair game, and the incorrect warning is not emitted by us, closing as it seems there's nothing to be done on Node.js side.

@aduh95 aduh95 closed this as not planned Won't fix, can't repro, duplicate, stale Dec 20, 2023
@everett1992
Copy link
Contributor Author

everett1992 commented Dec 23, 2023

Thanks for the well thought out response, I understand closing the issue, I figured this was caused by undefined or experimental behaviors.

Do you have any pointers for the issues we see in Jest?

    TypeError: Cannot assign to read only property 'performance' of object '[object global]'

@ilKhr
Copy link

ilKhr commented Dec 24, 2023

I solved it in the following way

https://stackoverflow.com/a/77694958/14274230

@juanito-caylent
Copy link

Do you have any pointers for the issues we see in Jest?

    TypeError: Cannot assign to read only property 'performance' of object '[object global]'

This is still a breaking change that didn't affect Node 18.18 and earlier, with no experimental flags attached. I would either re-open this focusing only on this topic, or open a new issue.

derevnjuk added a commit to NeuraLegion/bright-cli that referenced this issue Jan 16, 2024
ihiroky added a commit to ihiroky/live-comment that referenced this issue Feb 10, 2024
ihiroky added a commit to ihiroky/live-comment that referenced this issue Feb 11, 2024
* fix: Use npx, not yarn

* fix: Update common, server

* fix: Update comment

* chore: Update dependencies

* fix: Modify tests for poll

* fix: Use styled components

* fix: Update extention

* fix: update settings

* chore: Add package-lock.json

* fix: Use npx instead of yarn

* fix: Use npm to execute script

* fix: call eslint and jest through npm script

* fix: Use npm scripts

* fix: Use node 18.16.0

nodejs/node#51048

* fix: Fix type check

* fix: Update rollup-typescript plugin

* fix: Update rollup

rollup/rollup#4213

* fix: Update entry point

* fix: Add workaround for hung up on windows-latest

* fix: Add windows hungup workaround
victorshevtsov added a commit to victorshevtsov/verifiable-data-streams that referenced this issue May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants