-
Notifications
You must be signed in to change notification settings - Fork 26.1k
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
Edge Functions silently falling back to Nodejs Runtime #38743
Comments
I am able to reproduce the bug. I suspect it is caused by Next.js' built-in static analysis (The In the meantime, you can work around it by changing the quote: export const config = {
- runtime: `experimental-edge`
+ // change from ` to '
+ runtime: 'experimental-edge'
} Update
I have located the issue. It seems that Next.js is not able to statically extract value from Update I have submitted a PR. |
Incredible. I've been using backticks for years and this is the first time I've ever encountered a bug because of them. Explains why I was having such a hard time putting together a reproduction too. My minimal isn't using the eslint ruleset I typically use in my projects, so it wasn't auto-fixing the config string to use backticks. Explains why when I copied the code from the original broken project over to the minimal that it broke. Thanks for implementing a fix! Again I can't believe it came down to something as mundane as quotes. |
In Next.js, Although unable to handle |
I'd need to see some convincing perf data to change my mind about this one. Everything I've read up to now suggests either the opposite is true or that the difference is so small (<1%) between browser environments that it doesn't matter. The perf cost adds up with the more interpolations there are in your string, so in a case such as this, where there are no interpolations, you'd never see that overhead accrue. Perf concerns aside, while I may be an outlier here in terms of my preference for backticks, it doesn't seem unreasonable to me that others could encounter the same problem and be caught unaware of the internal behavior Nextjs has for evaluating config settings. I'm fully aware that they are different data types, but it's difficult to know exactly when that distinction is going to make a difference, as it so rarely does in this case. From the user perspective, if it's valid JS then it should just work. At best, it could be inferred that an exported config object will be evaluated as JSON and if it's not serializable as such, then it could have unexpected behavior. Of course, documentation would clear up this ambiguity. Might be worth considering, if it does not exist already, adding a |
## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have helpful link attached, see `contributing.md` Fixes #38743. Fixes: #38750 The PR adds basic `TemplateLiteral` support for static analysis. The corresponding re-production of #38743 has also been implemented in e2e tests.
This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you. |
Verify canary release
Provide environment information
What browser are you using? (if relevant)
N/A
How are you deploying your application? (if relevant)
Vercel
Describe the Bug
I first noticed this behavior when upgrading a Nextjs 12.1 project to 12.2, having followed the migration guide to convert my Nodejs API routes to the new Edge function runtime. Quickly I found out that my routes weren't behaving as expected, as I was getting an error from the following line:
Here I am following the recommendation from the middleware migration guide here (Please note, it has been mentioned in the docs that the Edge Runtime and Middleware share the same function signature and runtime API). This pattern is also mentioned in the official Edge Functions docs. This results in a runtime error because
req.url
is not a complete url, it's only the pathname.This lead me to investigate further and so I put together a minimal edge route to log out the
NextRequest
andNextFetchEvent
arguments to see what I was getting back from them:In the minimal reproduction I've provided, running
yarn dev
and then visitinglocalhost:3000/api/edge
will lead to the following to be logged in your terminal:As you can see, clearly we're not getting the expected data types here. The above matches what you would expect from the standard Nodejs runtime, where
NextRequest
extendsIncomingMessage
andNextResponse
extendsServerResponse
. Additionally as you can see,req.url
was logged as/api/edge
, which explains why the code I wrote earlier to extract request parameters resulted in a runtime error.From here the next steps I tried were to run
yarn why next
to determine if I hadn't actually upgraded to 12.2. Nothing appeared to be wrong there. So to be safe, I nuked mynode_modules
, yarn lockfile, etc and reinstalled. Same issue. Another developer suggested I try using another 12.2 feature to verify that I was indeed running it, so I enablednext/future/image
in mynext.config.js
, which you can see from the console output above I'm properly getting warned about on startup.It was from this point I started putting together a minimal, because as far as I can tell I'm doing all I need to in order to opt into using the Edge runtime. With that together, I was able to reproduce this failure on two additional machines (Windows 10 and macOS Monterey) as well as inside of GitHub Codespaces and asking another engineer to confirm.
Expected Behavior
I just want to be able to use the Edge runtime for my API routes and have a clear understanding of why the documented configuration instructions aren't working. 😭
Link to reproduction
https://github.com/Saeris/edge-fail
To Reproduce
Steps to Reproduce
yarn
yarn why next
yarn dev
{ req: IncomingMessage { ... }, event: ServerResponse { ... }, url: '/api/edge' }
The text was updated successfully, but these errors were encountered: