Skip to content
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

Error with client using Vercel Edge Function #14

Closed
flayks opened this issue Jun 16, 2022 · 13 comments
Closed

Error with client using Vercel Edge Function #14

flayks opened this issue Jun 16, 2022 · 13 comments

Comments

@flayks
Copy link

flayks commented Jun 16, 2022

Vercel Edge Functions are still in beta but that sounds like a Sanity issue here: I'm running a SvelteKit project using the Sanity Client to make queries on Page Endpoints through a function, and this is the error that I get from the Vercel logs:

Error: Dynamic require of "url" is not supported
    at worker.js:274:21872
    at node_modules/.pnpm/eventsource@2.0.2/node_modules/eventsource/lib/eventsource.js (worker.js:324:7541)
    at worker.js:274:22035
    at node_modules/.pnpm/@sanity+eventsource@4.0.0/node_modules/@sanity/eventsource/node.js (worker.js:325:1640)
    at worker.js:274:22035
    at node_modules/.pnpm/@sanity+client@3.3.2/node_modules/@sanity/client/lib/data/listen.js (worker.js:325:2969)
    at worker.js:274:22035
    at node_modules/.pnpm/@sanity+client@3.3.2/node_modules/@sanity/client/lib/data/dataMethods.js (worker.js:325:5186)
    at worker.js:274:22035
    at node_modules/.pnpm/@sanity+client@3.3.2/node_modules/@sanity/client/lib/sanityClient.js (worker.js:331:24738)

More infos: https://vercel.com/docs/concepts/functions/edge-functions
Possible limitations (Node related?): https://vercel.com/docs/concepts/functions/edge-functions/middleware-api#unsupported-apis-and-runtime-restrictions

@flayks
Copy link
Author

flayks commented Jun 16, 2022

Just had an idea: is it maybe because the client uses require over import and needs to be using ESM?

@stipsan
Copy link
Member

stipsan commented Aug 27, 2022

We're actively working on supporting both ESM and new runtimes like vercel edge functions, netlify deno workers and more.

You can test the new client using this command: npm i @sanity/client@esm and keep tabs on our progress here: #29

@stipsan
Copy link
Member

stipsan commented Feb 2, 2023

Hi, could you try with npm i @sanity/client@^5.0.0 to see if it resolves it?

@flayks
Copy link
Author

flayks commented Feb 10, 2023

Hi, could you try with npm i @sanity/client@^5.0.0 to see if it resolves it?

Seems fine! Cannot really say since when but haven't got this error for a while.

@siffogh
Copy link

siffogh commented Feb 20, 2023

I'm facing a different problem when deploying on the Vercel edge functions:

Error: The Edge Function "404" is referencing unsupported modules:
- follow-redirects: url, http, https, stream, assert
--
10:20:03.518 | - from2: events, util, stream
10:20:03.518 | - get-it: http, https, querystring, url, stream, zlib
10:20:03.518 | - through2: events, stream, util
10:20:03.518 | - tunnel-agent: net, tls, http, https, events, assert, util, buffer

Tested with the @sanity/client@^5.2.1

@narration-sd
Copy link

@stipsan, Hello - on Netlify (this time) Edge functions, the 'url' fix you gave of using @sanity/client@5.0.0 appears to work; at least it compiles.

But with it, I can't get further because now there is a similar fail for the 'http' function, messages below.

Would you like me to make another issue for the get-it module where apparently 'http' problematically appears, as that is Sanity's also, or is it sufficent to raise the point here?

Thanks,
Clive

Netlify log fragment:

9:39:56 PM: 05:39:56 AM [build] Building server entrypoints...
9:39:56 PM: [commonjs--resolver] Cannot bundle Node.js built-in "http" imported from "node_modules/get-it/dist/middleware.cjs". Consider disabling ssr.noExternal or remove the built-in dependency.
9:39:56 PM: file: /opt/build/repo/astro/node_modules/@sanity/client/dist/index.cjs
9:39:57 PM:  error   Cannot bundle Node.js built-in "http" imported from "node_modules/get-it/dist/middleware.cjs". Consider disabling ssr.noExternal or remove the built-in dependency.
9:39:57 PM:   File:
9:39:57 PM:     /opt/build/repo/astro/node_modules/@sanity/client/dist/index.cjs
9:39:57 PM:   Stacktrace:
9:39:57 PM: RollupError: Cannot bundle Node.js built-in "http" imported from "node_modules/get-it/dist/middleware.cjs". Consider disabling ssr.noExternal or remove the built-in dependency.
9:39:57 PM:     at error (file:///opt/build/repo/astro/node_modules/rollup/dist/es/shared/rollup.js:2091:30)

@narration-sd
Copy link

narration-sd commented Feb 23, 2023

@stipsan -- happened across this link when discovering about the problem initially, and it has good discussion w/code for doing the equivalent of http listen in Deno.

Much better than what seems to be needed in the other direction of conversion! And so I'm thinking it would just take some conditional code after recognizing whose edge service you were on. Maybe just testing a dynamic include could say??

@narration-sd
Copy link

narration-sd commented Feb 24, 2023

@stipson, I looked into this a little farther, and think I have a fix for you, low risk and very easy, if you agree.

In fact, we could wonder what npm module http is doing in the Sanity client at all -- and in fact, it's only in a test fixture, thus won't be needed in the published @sanity/client.

I didn't spot where you may have removed url for 5.0.0, but perhaps it was also such a case. Anyway, test code would be a likely place for libraries which wouldn't have a Deno equivalent, even in their now-provided npm somewhat-compatibilty set..

There's actually an http ability in there, and if it were functional enough could be selected by using a little code based on dynamic package imports, but this is surely not an appropriate effort here, is it?

So I propose that you simply tell the publishing step not to include the /text folder at all. This is one more kind of fraught business, but in this case the simplest seems to be to use an .npmignore file at the root of the sanity/client repo.

Because that file completely blanks npm's automatic ignoring of what's in .gitignore, you'd need to copy .gitignore's contents into .npmignore, then add a test/ line to remove the test folder from the publish. Quite likely with a comment in both files to say how they need to be kept in sync, not losing the extra ignore for test folder.

I could do a PR for this if you'd like -- have stopped here short of messing with my own publish in the npm namespace, which would be necessary to prove this for Netlify's build....

@stipsan
Copy link
Member

stipsan commented Mar 13, 2023

Hmmm it seems like the bundling for Vercel's Edge is somehow ending up with the node version of the client instead of the build that uses fetch.
There are two pieces to this though. @sanity/client itself uses get-it. Both have pkg.exports defined that should be targeted by the build.

Could you share more information on your bundler setup?

In fact, we could wonder what npm module http is doing in the Sanity client at all -- and in fact, it's only in a test fixture, thus won't be needed in the published @sanity/client.

It's not just in test fixtures. The http module is used when the client is bundled for a node env:

The Vercel Edge runtime is part of our E2E test suite:

If you can share your setup I'd love to add it to the rig @narration-sd :) And PRs are of course welcome!

@hdoro
Copy link

hdoro commented Mar 23, 2023

@stipsan I've just tried the new @vercel/remix integration (see announcement post), and @sanity/client@5.3.2 works properly there in both edge and serverless environments 🎉

@stipsan
Copy link
Member

stipsan commented Mar 23, 2023

@hdoro that's great news! ✨

@hdoro
Copy link

hdoro commented Mar 28, 2023

@stipsan I need to correct that: I don't know how it worked on the successful project I mentioned above, but new projects aren't working. I've tried 5.4.0 and logging unstable__adapter, unstable__environment I get node:

2023-03-28-Firefox Developer Edition@2x

For now I'm replacing my only instance of @sanity/client with picosanity, and it's working. I've kept a type import to @sanity/client, and Vercel is fine with that, though.

@stipsan
Copy link
Member

stipsan commented Mar 28, 2023

@hdoro That's unfortunate indeed. unstable__environment should be browser and unstable__adapter should be xhr.
We do run extensive tests both for get-it and @sanity/client in the Edge Runtime. The problems we've seen so far isn't so much the runtime itself, but rather that the host bundling somehow ends up including the node version of get-it and @sanity/client instead of the browser based one.

The issues we've been seeing are so far consistently bundling related. For whatever reason the node environment of our packages are used instead of the ones we want.

@stipsan stipsan closed this as completed Jan 2, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants