Add register.js for use with --import flag #4084
Conversation
with --import flag
@rochdev Cheers. Which test is the "loader test" you refer to? 
|
Looks like it was removed, but basically a file similar to It also looks like some of the CI Visibility tests are using the old approach, but I don't think this is a problem in the short-term. |
|
@rochdev I opted for wrapping I also noticed that adding an
...so I removed it to not risk making a breaking change. |
There was a problem hiding this comment.
I recommend viewing this with whitespace hidden:
https://github.com/DataDog/dd-trace-js/pull/4084/files?diff=split&w=1
| it('works for options.port', async () => { | ||
| proc = await spawnProc(startupTestFile, { | ||
| cwd, | ||
| execArgv, |
There was a problem hiding this comment.
Alternatively, env.NODE_OPTIONS could be set below. I'm not sure which approach is preferred. execArgv seems more correct but both approaches seem to run the loader.
smartinio
changed the title
|
Resubmitted as #4110 so it can run the full CI. |
What does this PR do?
Add support for using
node --import dd-trace/register.jsNote: If you want to support extensionless import (dd-trace/register), package.json needs to explicitly list it in
exports. However, adding anexportskey is a breaking change which I would not recommend doing at this stage.Fixes #3794 (I think)
Motivation
--loader(aka--experimental-loader) is deprecated and using--importandregisteris the officially recommended way by Node.jsSee (these anchors seem broken, but you'll figure out which heading it is):
Additional Notes
Tried this out locally by changing my local node_modules to mirror these changes
Security
Datadog employees:
@DataDog/security-design-and-guidance.Unsure? Have a question? Request a review!