Upgrade @vercel/nft to 0.27.4 to include ESM module.register hooks

[s1gr1d]

Environment

ESM

Reproduction

Link: https://github.com/s1gr1d/nitro-sentry-iitm

This is how import-in-the-middle (IITM) looks like in .output/server/node_modules:

Without resolution (uses @vercel/nft 0.26.x)	With @vercel/nft package resolution to 0.27.4

Describe the bug

Previous versions of @vercel/nft do not include ESM module.register hooks so the @opentelemetry/instrumentation/hook.mjs is not included, which leads to errors when starting the server if opentelemetry or packages that depend on it (like Sentry) are used.

---

@vercel/nft released a new version with the fix, but as it is still a 0 major version, the caret (^) does not work like usual. With 0 major versions, only the patch is automatically updated, but not the minor (npm source).

The current release of nitro depends on ^0.26.5 and the fix is included in 0.27.4 (another minor version).

It would be awesome to release a new nitro version (minor or patch), which includes this @vercel/nft minor version upgrade 🙌

Additional context

Issue Reference:

• Does not include registered ESM loader hooks vercel/nft#428
• Problems with @sentry/astro on Netlify getsentry/sentry-javascript#12603 (netlify-related, but this uses nft as well) - includes a reproduction

Logs

No response

[pi0]

We have already upgraded to 0.27 range in v2/main branches but since v2 branch has some risky changes that require more ecosystem testing, unfortunately, there is no immediate release planned.

In the meantime, it is possible to use nightly release channel to receive the update.

[s1gr1d]

Thanks for this information :) Would it be possible to create a patch release (nitro 2.9.8) with just this nft change? The current 0.27 range is fine as well, but the current version 2.9.7 only includes 0.26. I understand that this might be a big request but with this update people would be able to use OTel instrumentation with nitro (OTel uses import-in-the-middle).

[pi0]

We can... Can you please help to make a minimal nitro reproduction for the issue with open telemetry dependency? I remember last time there were more issues specially on nodeless (edge) platforms this way at least we can make sure this releases fully solves the problem.

[s1gr1d]

I updated the issue and created a minimal reproduction with Sentry as Sentry uses module.register to add import-in-the-middle to make it work with OTel: https://github.com/getsentry/sentry-javascript/blob/a8e5f593b3341f90cf9aafaaa1f6eb1e0da7545e/packages/node/src/sdk/initOtel.ts#L68

[andreiborza]

Hi @pi0, any decisions on this? This also holds up otel instrumentation for vinxi for example.

[pi0]

Thanks for reproduction @s1gr1d it helped me to understand the better of usage.

@andreiborza We are working on the v2 branch to be stable (we also already asked both Nuxt and solid/vinxi teams since last week to review the nightly channel) as soon as we are sure of stability will issue a new release, I hope (but don't promise) by early next week at latest for it to happen.

My understanding is that import-in-the-middle is an experimental feature (and it is for Node.js targets only). In the meantime (while I fully understand the inconvenience) you can offer users to opt into the nightly channel or manually add the resolution of vercel/nft.

On a relevant topic, if you are interested I would love to have better support of Sentry for Nitro (both Node.js and Edge) (long-time user!), i will contact you if you are intrested.

[s1gr1d]

Hy, yeah adding a resolution is the only current workaround. And it is good to hear that another release is coming soon! Currently, the Nuxt and SolidStart SDKs are in alpha/beta and it would be nice for developers to use it without the resolution once the proper release of the SDK comes out.

And we are about to start working on a Nitro instrumentation: getsentry/sentry-javascript#13670
It's great to hear that we can reach out to you!

[pi0]

Sounds exciting. I could not find any contact in X. Would you mind to drop me a DM in discord (@pi0) or X (@_pi0_)? 🙏🏼

[cjpearson]

We've been using a manual OpenTelemetry instrumentation for our projects and the idea of having an officially supported version is exciting!

Have you given any thought to supporting Native Instrumentation within the Nitro package itself? (Similar to NextJS) With an external package it can be difficult to inject the necessary code at the right points. For example, in our instrumentation package we have to use a nitro plugin where we get a handle on the h3 app and patch the nitro router to wrap each request in a handle. It works, but as with all code that depends on internals there is the risk of future breakage.

https://opentelemetry.io/docs/concepts/instrumentation/libraries/

[pi0]

@cjpearson Actually yes one of the reasons I am thinking of nitro-native support is to have a cross-platform instrument system. I don't prefer the complexity of open telemetry Node.js SDK either.

[cjpearson]

Oh cool. Are you open to PRs in this direction?

[DeVoresyah]

Hi, I'm using Nuxt 3 and already add resolutions for @vercel/nft but still didn't work. Here's my package version:

"nuxt": "^3.9.0",

[pi0]

2.10 is released with vercel/nft dependency update.

otel support requires more works.