Resolving files with querystring

[lforst]

Hi, I am one of the maintainers of the @sentry/nextjs SDK and we recently got a bug report stating that we mess up the output of @vercel/nft. This is completely valid and reproducible.

What we're doing in the SDK is that we have a webpack loader that "proxies" user files and re-exports wrapped versions of particular functions (i.e. Next.js data fetching methods and API routes).

For the proxying mechanism we first replace the user-file with a proxy-file, from which we then import the user-file with a query string at the end to mark it as "already proxied" to avoid infinite loops (e.g. import * as wrapee from './page.js?__sentry_wrapped__'). The rollup docs explain this rather well.

For some reason this breaks the Next.js' nft file generation, leading to my first question: Do you know why this would be happening?

Webpack is able to resolve imports containing a query parameters - it simply strips the query parameter when looking for the file in the file system. Is nft also able to resolve these paths? I tried using nft why to experiment but it seemed like it couldn't resolve it with the error:

Error: Failed to resolve dependency ./pages/index.js?__sentry_wrapped__:
Cannot find module '/workspaces/sentry-nextjs-vercel-testproject/pages/index.js?__sentry_wrapped__' loaded from /workspaces/sentry-nextjs-vercel-testproject/test.js

Do you know if there is a workaround for us or something we can do here? We would also be ready to contribute upstream to support query params and fragments in import paths.

[styfle]

Do you know why this would be happening?

Looks like a bug. I confirmed Node.js will drop query string params for ESM imports (not CJS) so we need to fix @vercel/nft to resolve the same way.

We would also be ready to contribute upstream to support query params

Sounds good! Let me know when you have a PR ready and I'll take a look, thanks!

[lforst]

Thanks for taking a look! Just to make the process of fixing this a bit quicker would you be able to give a pointer where that logic would have to be added?

[styfle]

I don’t know off the top of my head. I would start by adding a failing unit test and look at the stack trace when the error is thrown.

[lobsterkatie]

Just chiming in as another one of the folks working on @sentry/nextjs.

I can't seem to replicate the error Luca mentioned above, either with ESM or CJS files, and with either yarn nft why or yarn nft print. That said, I did just push a PR fixing the problem which led to this issue being filed in the first place.

I would start by adding a failing unit test

I mentioned this in that PR, too, but I couldn't get a handle on how exactly one would do that. Would love some help in that regard, please.