fix: removed support for experimental loader flag #2153
Conversation
aryamohanan
force-pushed
the
v5-release
branch
from
66f40a8 to
0b41e6e
Compare
aryamohanan
force-pushed
the
fix-drop-esm-loader
branch
from
af69d60 to
70826e1
Compare
| if (esmUtil.hasExperimentalLoaderFlag()) { | ||
| // eslint-disable-next-line no-console | ||
| console.error( | ||
| 'Node.js introduced breaking changes in versions 18.19.0 and above, leading to the discontinuation of support ' + |
There was a problem hiding this comment.
suggestion:
introduced breaking changes
IMO this is too much information without a link to their breaking change.
Thats why IMO:
For Node.js versions later than 18.19, the --experimental-loader flag and ESM support are no longer available.
|
|
||
| // This loader worked with '--experimental-loader' in Node.js versions below 18.19. | ||
| // TODO: Remove 'esm-loader.mjs' file and this log in the next major release (v6). | ||
| if (esmUtil.hasEsmLoaderFile()) { |
There was a problem hiding this comment.
The use case here is:
--import esm-loader.mjs
Right?
Its very nice to catch the wrong usage - especially because we drop the syntax.
I feel like the error message is not on point?
|
We need a todo to cleanup task for internal Github usages of "--experimental-loader". |
| // eslint-disable-next-line max-len | ||
| `The package @instana/aws-fargate requires at least Node.js ${nodeJsVersionCheck.minimumNodeJsVersion} but this process is ` + | ||
| `running on Node.js ${process.version}. This Fargate container will not be monitored by Instana.` + | ||
| 'See https://www.ibm.com/docs/en/instana-observability/current?topic=agents-aws-fargate#versioning.' |
There was a problem hiding this comment.
https://www.ibm.com/docs/en/instana-observability/current?topic=agents-aws-fargate#versioning
Is this still a valid link? 🤔
If not, its probably good to fix that straight on main branch
There was a problem hiding this comment.
Yeah it does, but somehow I cant follow we link there specifically 🤔
| console.error( | ||
| `The package @instana/aws-fargate requires at least Node.js ${minimumNodeJsVersion} but this process is ` + | ||
| // eslint-disable-next-line max-len | ||
| `The package @instana/aws-fargate requires at least Node.js ${nodeJsVersionCheck.minimumNodeJsVersion} but this process is ` + |
There was a problem hiding this comment.
If a customer uses < 18.19 and the legacy esm loader, he will first run into this warning. Then he updates the node version, restarts and then he runs into the next warning.
I am wondering if we should already link to the migration? 🤔 Because we currently only link to agents-aws-fargate#versioning.
| "Node.js 18.19.0 and later no longer support the '--experimental-loader' flag for ESM. " + | ||
| `Your current version is ${process.version}. To ensure tracing by Instana, ` + | ||
| "please use the '--import' flag instead. For more information, refer to the Instana documentation: " + | ||
| 'https://www.ibm.com/docs/en/instana-observability/current?topic=nodejs-collector-installation.' |
There was a problem hiding this comment.
Should we link to the upcoming v4->v5 migration guide?
| if (esm.hasEsmLoaderFile()) { | ||
| // eslint-disable-next-line no-console | ||
| console.error( | ||
| "Importing 'esm-loader.mjs' is not supported and will be removed in next major release. " + |
There was a problem hiding this comment.
| "Importing 'esm-loader.mjs' is not supported and will be removed in next major release. " + | |
| "Importing 'esm-loader.mjs' is not a valid command. " + |
BREAKING CHANGE
Notes:
--experimental-loaderis already not working for Node versions >= v18.19.0 (documented already)esm-loader.mjsworks with--experimental-loaderonly.We are dropping support for < v18.19.
Reference
Autotrace webhook: https://github.ibm.com/instana/autotrace-mutating-webhook/pull/670