-
Notifications
You must be signed in to change notification settings - Fork 87
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
fix: add AsyncLocalStorage
to globalThis
#1907
Conversation
✅ Deploy Preview for netlify-plugin-nextjs-nx-monorepo-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-static-root-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-export-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-next-auth-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-i18next-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-canary ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for nextjs-plugin-custom-routes-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-edge-middleware ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
it(`${label}should fail when returning a stream`, async () => { | ||
it(`${label}should not fail when returning a stream`, async () => { | ||
const res = await fetchViaHTTP(next.url, `${locale}/stream-a-response`) | ||
expect(res.status).toBe(500) | ||
expect(res.status).toBe(200) | ||
|
||
if (!(global as any).isNextDeploy) { | ||
expect(await res.text()).toEqual('Internal Server Error') | ||
expect(next.cliOutput).toContain( | ||
expect(next.cliOutput).not.toContain( | ||
`A middleware can not alter response's body. Learn more: https://nextjs.org/docs/messages/returning-response-body-in-middleware` | ||
) | ||
} | ||
}) | ||
|
||
it(`${label}should fail when returning a text body`, async () => { | ||
it(`${label}should not fail when returning a text body`, async () => { | ||
const res = await fetchViaHTTP(next.url, `${locale}/send-response`) | ||
expect(res.status).toBe(500) | ||
expect(res.status).toBe(200) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are test changes from upstream
Also for more context, this is to mirror what they're doing in their repo. // expose AsyncLocalStorage on globalThis for react usage
const { AsyncLocalStorage } = require('async_hooks')
;(globalThis as any).AsyncLocalStorage = AsyncLocalStorage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -0,0 +1,11 @@ | |||
'use client' | |||
|
|||
// TODO-APP: support typing for useSelectedLayoutSegment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an issue for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a Next.js demo site, not our code
|
||
// TODO-APP: support typing for useSelectedLayoutSegment | ||
// @ts-ignore | ||
import { useSelectedLayoutSegments } from 'next/navigation' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Summary
Since version
13.1.2
, Next.js has requiredAsyncLocalStorage
to be available on the global object for edge SSR. Deno, being excellent people, have now added an implementation to their node compat library, so this adds that toglobalThis
.Test plan
/app-edge
in the default demoRelevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal
Fixes #1851
Standard checks:
🧪 Once merged, make sure to update the version if needed and that it was published correctly.