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

unstable_serialize doesn't work with NextJS edge runtime #3030

Open
william1616 opened this issue Oct 23, 2024 · 2 comments
Open

unstable_serialize doesn't work with NextJS edge runtime #3030

william1616 opened this issue Oct 23, 2024 · 2 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@william1616
Copy link

william1616 commented Oct 23, 2024

Bug report

Description / Observed Behavior

When calling unstable_serialize from a server component that uses the NextJS edge runtime with an array or object key, the key is not serialized properly.

As an example, the code below logs 1~ to the console:

export const runtime = 'edge'

export default function Home() {
  const key = ['test']
  console.log(unstable_serialize(key))

  return "Hello"
}

Expected Behavior

I would expect the behavior in edge runtime to match the NodeJS runtime, with @"test", logged to the console (example code below)

export default function Home() {
  const key = ['test']
  console.log(unstable_serialize(key))

  return "Hello"
}

Additional Context

Debugging

I've done some digging, and the problem seems to originate from the stableHash function in the file: https://github.com/vercel/swr/blob/v2.2.5/src/_internal/utils/hash.ts.

  • Line 40 (if (constructor == Array) {) and Line 48 (if (constructor == OBJECT) {) don't return true for arrays/objects when using the edge runtime.

Further testing reveals that although key.constructor doesn't equal Array in the edge runtime, key instanceof Array returns true (same applies to objects).

A possible solution could be to change:

  • Line 40 to if (arg instanceof Array) {
  • Line 48 to if (arg instanceof OBJECT) {

This works in both the edge runtime and NodeJS runtime, but I'm unsure of the historical reasons for not using instanceof. Additionally, I'm not sure why arg.constructor doesn't equal Array or OBJECT in the edge runtime.

Package Versions

[email protected]
├── @types/[email protected]
├── @types/[email protected]
├── @types/[email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
└── [email protected]

Repro Steps / Code Example

I've built a test repository that demonstrates the issue. To reproduce:

  1. Clone the repo from https://github.com/william1616/nextjs-swr-edge-runtime-tests.
  2. Install the packages (npm i), and then run the development server (npm run dev).
  3. Open a browser and navigate to http://localhost:3000.
  4. Check the console output:
    • 1~ (the output of unstable_serialize).
    • true (key instanceof Array is true).
    • false (key.constructor == Array is false).
  5. In https://github.com/william1616/nextjs-swr-edge-runtime-tests/blob/main/src/app/page.tsx, comment out line 5 to change the runtime from edge to NodeJS.
  6. Refresh http://localhost:3000 in your browser.
  7. Check the console output:
    • @"test", (the output of unstable_serialize).
    • true (key instanceof Array is true).
    • true (key.constructor == Array is true).
@promer94 promer94 added bug Something isn't working help wanted Extra attention is needed labels Oct 23, 2024
@william1616
Copy link
Author

Follow-up

Having done a bit more testing, the reason for using .constructor rather than instanceof is because types like new Set are subclasses of object, and using instanceof would cause subclasses of object to be serialized as objects rather than being added to the weak map.

So my new suggested solution is to use Object.getPrototypeOf — which seems to work in the edge runtime. I'm still unclear why .constructor doesn't work in the edge runtime, though.

Here are some examples:

console.log({}.constructor == Object) // false
console.log([].constructor == Array) // false

console.log({} instanceof Object) // true
console.log([] instanceof Array) // true
console.log(new Set([1, 2]) instanceof Object) // true (we want this to be false so it's not serialized as an object)

console.log(Object.getPrototypeOf({}) == Object.prototype) // true
console.log(Object.getPrototypeOf([]) == Array.prototype) // true
console.log(Object.getPrototypeOf(new Set([1, 2])) == Object.prototype) // false (this won't be serialized as an object)

@promer94
Copy link
Collaborator

promer94 commented Oct 23, 2024

Thanks for your awesome reproduction ❤️

The instanceof works because edge-runtime uses proxy to patch the behavior https://github.com/vercel/edge-runtime/blob/d6cb77fba517c2e55b9c0353b43f5f2d50c466a2/packages/vm/src/edge-vm.ts#L120

I'm still unclear why .constructor doesn't work in the edge runtime, though.

the reason why the constructor of [] is not globalThis.Array is because vercel/next.js#38184 (comment)

promer94 added a commit that referenced this issue Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants