-
Notifications
You must be signed in to change notification settings - Fork 400
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
fix: Fixed issue with CJS being imported as ESM #2168
Conversation
7a53d5d
to
e3a4227
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall makes sense and looks good. a few comments
lib/is-absolute-path.js
Outdated
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
'use strict' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'd put this in lib/util
t.context.agent = agent | ||
|
||
const { default: foo } = await import('./foo.cjs?v=' + crypto.randomBytes(16).toString('hex')) | ||
t.context.mod = foo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should you just import this and then access all 3 different exports and verify it works as expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think I understand the question. I have to import it after the "agent" is setup because otherwise the registerHooks
will not pick up the module.
node-newrelic/test/lib/agent_helper.js
Lines 180 to 190 in cb21d2c
helper.instrumentMockedAgent = (conf, setState = true, shimmer = require('../../lib/shimmer')) => { | |
shimmer.debug = true | |
const agent = helper.loadMockedAgent(conf, setState) | |
shimmer.bootstrapInstrumentation(agent) | |
shimmer.registerHooks(agent) | |
helper.maybeLoadSecurityAgent(agent) | |
return agent | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh so i meant importing the file to get at default
, foo
and just the raw module.exports. Looking at this right now you're just getting default correct?
This PR resolves #2155. This PR supercedes #2158.