instead of patching the tracer.js file to throw on @opentelemetry/api imports, delete the @opentelemetry/api dependency itself

[dario-piotrowicz]

the problem that this addresses is that the @opentelemetry/api package is not only imported by the tracer.js file
we patch, so just deleting the library itself makes sure that all files requiring it get the same throwing behavior
(besides decreasing the overall worker size)

[changeset-bot]

🦋 Changeset detected

Latest commit: 13a5f89

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@opennextjs/cloudflare	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[pkg-pr-new]

Open in Stackblitz

pnpm add https://pkg.pr.new/@opennextjs/cloudflare@259

commit: 13a5f89

[dario-piotrowicz]

In the apps I've tested my solution worked just fine (the one mentioned in the issue and my nodejs.org fork)

But in the create-next-app example is instead erroring 😓

(The apps were using npm but the create-next-app is under our pnpm monorepo, I wonder if that could causes this)

Anyways the issue is this code, usually the problematic require gets tree shaken away:

That doesn't seem to happen in the create-next-app's case though 😓

(I patched the node_modules file with a console log there just to get a worker outputted via wrangler deploy --dry-run --outdir, the original code is the one inside the console log)

I think this is extra problematic because as you can see, the file is inside the .next directory, outside of our .open-next output directory, reaffirming that we're still got some issues with the output directory not containing everything as mentioned in #14 😓

[dario-piotrowicz]

Given my comment above I am a bit of at a loss... if the tracer is outside of our output directory (.open-next) I think that there isn't much we can do about it... (I don't think we should touch/patch any files outside of our output directory)

It feels like for the time being (until we properly solve #14) we have to ask people to use wrangler aliases to make @opentelemetry/api requires throw...

Unless someone has some idea? @vicb, @petebacondarwin, @james-elicx? 😖

[vicb]

Not looked at it deeply but I was also wondering about

(The apps were using npm but the create-next-app is under our pnpm monorepo, I wonder if that could causes this)

Maybe with pnpm you will delete a link, resulting in the dep being deleted in the root node_modules while with npm you will only delete the local node_modules and the dep can still be resolved from the root.

[dario-piotrowicz]

while with npm you will only delete the local node_modules and the dep can still be resolved from the root.

Nono the dep is not resolved, I'm sure of it, I've debugged it multiple times and also that would break the applications (since the non-precompiled dep errors in workerd).

I do think that the dep deleting is correct, at least, if everything the worker needs (including node_modules) is supposed to be in .open-next then I do think that removing the dep there is the correct solution (since that's exactly what Next.js is checking: "is the opentelemetry dep available? if not use our precompiled version")

[nopsled]

Got any workaround for this?