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

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

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
@huozhi huozhi closed this as completed in bd839a4 Nov 25, 2024
alexandresoro pushed a commit to alexandresoro/ouca-backend that referenced this issue Dec 23, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [swr](https://swr.vercel.app) ([source](https://github.com/vercel/swr)) | dependencies | minor | [`2.2.5` -> `2.3.0`](https://renovatebot.com/diffs/npm/swr/2.2.5/2.3.0) |

---

### Release Notes

<details>
<summary>vercel/swr (swr)</summary>

### [`v2.3.0`](https://github.com/vercel/swr/releases/tag/v2.3.0)

[Compare Source](vercel/swr@v2.2.5...v2.3.0)

##### Feature

-   Support promises as fallback data by [@&#8203;shuding](https://github.com/shuding) in vercel/swr#2891
-   Allow to use with React 19 by [@&#8203;vladshcherbin](https://github.com/vladshcherbin) and [@&#8203;devjiwonchoi](https://github.com/devjiwonchoi) in vercel/swr#3047, vercel/swr#2963

##### Patches

-   fix [#&#8203;3030](vercel/swr#3030) and run relateive test in edge-runtime by [@&#8203;promer94](https://github.com/promer94) in vercel/swr#3036
-   fix: Only suspend when using the `fallback` by [@&#8203;shuding](https://github.com/shuding) in vercel/swr#3045
-   fix type check in tests by [@&#8203;huozhi](https://github.com/huozhi) in vercel/swr#3052
-   fix: Replace the deprecated 'window' with 'globalThis' for Deno by [@&#8203;saul-atomrigs](https://github.com/saul-atomrigs) in vercel/swr#2915
-   fix: check if config.fallback is undefined by [@&#8203;taku-hatano](https://github.com/taku-hatano) in vercel/swr#2913
-   fix(infinte): export SWRInfiniteKeyedMutator type by [@&#8203;LeoMcA](https://github.com/LeoMcA) in vercel/swr#2900
-   fix: Improve comparison performance by [@&#8203;shuding](https://github.com/shuding) in vercel/swr#2973
-   Export ScopedMutator type by [@&#8203;joshkel](https://github.com/joshkel) in vercel/swr#2937
-   Improve-Type-Safety-and-State-Access-in-useStateWithDeps-Hook by [@&#8203;O-BERNARDOFOEGBU](https://github.com/O-BERNARDOFOEGBU) in vercel/swr#3027

##### Misc

-   chore: bump dev deps and change example react version to latest by [@&#8203;huozhi](https://github.com/huozhi) in vercel/swr#2894
-   build: fix beta release job by [@&#8203;huozhi](https://github.com/huozhi) in vercel/swr#2895
-   chore: Improve test coverage by [@&#8203;shuding](https://github.com/shuding) in vercel/swr#2903
-   chore: simplify test coverage strategy by [@&#8203;huozhi](https://github.com/huozhi) in vercel/swr#2909
-   build: simplify react-server export and update bundler by [@&#8203;huozhi](https://github.com/huozhi) in vercel/swr#2897
-   examples: add RSC streaming pre-render with promise fallback example by [@&#8203;promer94](https://github.com/promer94) in vercel/swr#2905
-   Drop client-only by [@&#8203;huozhi](https://github.com/huozhi) in vercel/swr#2910
-   Mark package as side-effect free by [@&#8203;htunnicliff](https://github.com/htunnicliff) in vercel/swr#2904
-   Drop exports module field by [@&#8203;huozhi](https://github.com/huozhi) in vercel/swr#2911
-   chore: update pkg script watch by [@&#8203;unliar](https://github.com/unliar) in vercel/swr#2920
-   test: remove console.error times check by [@&#8203;promer94](https://github.com/promer94) in vercel/swr#2918
-   build: bump bundler for perf by [@&#8203;huozhi](https://github.com/huozhi) in vercel/swr#2929
-   Fix bundling of client entry chunks  by [@&#8203;huozhi](https://github.com/huozhi) in vercel/swr#2932
-   ci: fix ci error and upgrade action version by [@&#8203;promer94](https://github.com/promer94) in vercel/swr#2952
-   Add SWRInfiniteMutatorOptions type to export by [@&#8203;ludwigbacklund](https://github.com/ludwigbacklund) in vercel/swr#2954
-   test: update the revalidate function test for useSWRInfinite by [@&#8203;koba04](https://github.com/koba04) in vercel/swr#2955
-   chore: upgrade nextjs dev dep for e2e testing by [@&#8203;huozhi](https://github.com/huozhi) in vercel/swr#3044
-   ci: simplify ci config and bump some deps version by [@&#8203;promer94](https://github.com/promer94) in vercel/swr#2770
-   chore: reorganize entries by [@&#8203;huozhi](https://github.com/huozhi) in vercel/swr#3048
-   Bump bundler and reorganize serialize exports by [@&#8203;huozhi](https://github.com/huozhi) in vercel/swr#3049
-   upgrade use-sync-external-store to support react 19 by [@&#8203;huozhi](https://github.com/huozhi) in vercel/swr#3050

#### New Contributors

-   [@&#8203;htunnicliff](https://github.com/htunnicliff) made their first contribution in vercel/swr#2904
-   [@&#8203;saul-atomrigs](https://github.com/saul-atomrigs) made their first contribution in vercel/swr#2915
-   [@&#8203;unliar](https://github.com/unliar) made their first contribution in vercel/swr#2920
-   [@&#8203;ludwigbacklund](https://github.com/ludwigbacklund) made their first contribution in vercel/swr#2954
-   [@&#8203;taku-hatano](https://github.com/taku-hatano) made their first contribution in vercel/swr#2913
-   [@&#8203;LeoMcA](https://github.com/LeoMcA) made their first contribution in vercel/swr#2900
-   [@&#8203;devjiwonchoi](https://github.com/devjiwonchoi) made their first contribution in vercel/swr#2963
-   [@&#8203;vladshcherbin](https://github.com/vladshcherbin) made their first contribution in vercel/swr#3047
-   [@&#8203;O-BERNARDOFOEGBU](https://github.com/O-BERNARDOFOEGBU) made their first contribution in vercel/swr#3027

**Full Changelog**: vercel/swr@v2.2.5...v2.3.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS44Mi4zIiwidXBkYXRlZEluVmVyIjoiMzkuODIuMyIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIl19-->

Reviewed-on: https://git.tristess.app/alexandresoro/ouca/pulls/413
Reviewed-by: Alexandre Soro <[email protected]>
Co-authored-by: renovate <[email protected]>
Co-committed-by: renovate <[email protected]>
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