fix: support datadog-pprof #411
Conversation
|
Looks like we're hitting this Line 755 in c16e794 |
Yes, I described why this was the case in the issue here Datadog is using a CJS import and assigning it to a variable. So in the AST, we will not be able to infer the package name from the arguments (the arguments are I also find that having to manually resolve the arguments via checking the array of args a little hard-codey, so i decided to just have a special case catch all instead for the package to just include |
Could I also check if it would be possible to include a flag to ignore tree shaking modules in a package? I think it would make sense as a catch-all to handle special case like these! |
I have removed the sourcemaps outputs from |
|
I took a stab at fixing this properly in PR #419 |
- Fixes #410 - Closes #411 Note, this is easiest to review without whitespace https://github.com/vercel/nft/pull/419/files?w=1
Closes #410
Alternative solution considered
NODE_GYP_BUILDto account for problem identified in Add support for@datadog/pprof#410. This proved to be tricky as we need to retrieve the value fromfindBindingswhich involves a walk of the whole treeSolution
Instead of the alternative above, chose to just add a special case for this package
Automated tests
Just added a conditional
testName === '@datadog-pprof'to thefoundMatchingBinarystatement so we test the presence of theprebuildsfolder.Manual tests (since the automated tests do not test for presence of
prebuildsfolderRunning
node out/cli.js print node_modules/@datadog/pprof/out/src/index.jsgives me the following FILELISTWe see that the entire
prebuildsfolder is correctly included.