blitz-js / superjson

Safely serialize JavaScript expressions to a superset of JSON, which includes Dates, BigInts, and more.
MIT License
3.88k stars 83 forks source link

Parsing is incorrect with vercel edge runtime #242

Closed mattiaz9 closed 1 year ago

mattiaz9 commented 1 year ago

NextJS Example:

import SuperJSON from "superjson"

export const runtime = "edge"

export function GET() {
  const input =
    '{"json":{"page":1,"perPage":20,"orderBy":null,"sortDirection":null,"query":null},"meta":{"values":{"orderBy":["undefined"],"sortDirection":["undefined"],"query":["undefined"]}}}'

  const parsed = SuperJSON.parse<any>(input)

  return NextResponse.json(parsed)
}

In this example the fields with meta value undefined are set as null:

{
  "page": 1,
  "perPage": 20,
  "orderBy": null,
  "sortDirection": null,
  "query": null
}

Switching the runtime to nodejs results instead in a correct output:

{
  "page": 1,
  "perPage": 20
}
mattiaz9 commented 1 year ago

So.. i found the issue. In this code:

export const isPlainObject = (
  payload: any
): payload is { [key: string]: any } => {
  if (typeof payload !== 'object' || payload === null) return false;
  if (payload === Object.prototype) return false;
  if (Object.getPrototypeOf(payload) === null) return true;

  return (
    payload.constructor === Object &&
    Object.getPrototypeOf(payload) === Object.prototype
  );
};

payload.constructor === Object fails with edge runtime. Is that double check even necessary?

I tested it locally by evaluating the code with the edge-runtime npm package and by just using Object.getPrototypeOf(payload) === Object.prototype it works fine.

Skn0tt commented 1 year ago

Nice find! This check seems to be from the very beginnings of SuperJSON: https://github.com/blitz-js/superjson/commit/c617bc76a538dcdd182c0b31b08e2b3a7bce4ea0#diff-4c1f145257b72d7f61ed609a0604c966c68010147841b7d45f164c8ef3f8360aR15-R16

Let's try to remove the .constructor check, yes. @mattiaz9 do you want to open a PR for this? that'd be lovely :D

mattiaz9 commented 1 year ago

sure!

Skn0tt commented 1 year ago

Fix released in https://github.com/blitz-js/superjson/releases/tag/v1.12.4

Skn0tt commented 1 year ago

cc @KATT