Custom classes will crash if using `accessor`s because created with `Object.create` instead of `new Class`

[pie6k]

Currently, when registering custom class with registerClass, and then deserializing, new instance of the class is created with:

(https://github.com/flightcontrolhq/superjson/blob/bc7a825a6cb51934ed91979366830927244ff98d/src/transformer.ts#L281)

It leads tonon-trivial issues, eg code below will crash with error:

TypeError: Cannot write private member #bar_accessor_storage to an object whose class did not declare i

Crashing code:

const superjson = new SuperJSON();

class Foo {
  accessor bar!: string; // https://github.com/tc39/proposal-grouped-and-auto-accessors (stage 3)
}

superjson.registerClass(Foo, {
  identifier: "Foo",
  allowProps: ["bar"],
});

const foo = new Foo();
foo.bar = "foo";
const encoded = superjson.serialize(foo);

console.dir(encoded, { depth: null });

console.log("will deserialize");
const decoded = superjson.deserialize(encoded);
console.dir(decoded, { depth: null });

If we change https://github.com/flightcontrolhq/superjson/blob/bc7a825a6cb51934ed91979366830927244ff98d/src/transformer.ts#L281

into

Object.assign(new clazz(), v)

it seems to work. Of course it then has to assume constructor accepts no arguments

[Skn0tt]

Hi! The linked proposal claims to be in Stage 1, you mentioned it's in Stage 3. https://github.com/tc39/proposals#active-proposals doesn't even list it. Could you double-check the stage of the proposal? I'm willing to look into support for Stage 3, but Stage 1 not so much.

[pie6k]

Hey!

That's true, I did see Stage 3 at https://github.com/tc39/proposal-decorators instead and it mentions accessors in the first section.

Regardless of this, it will also fail with # private fields (as accessors are just syntax sugar over it), eg this will also fail with exactly the same error:

  const superjson = new SuperJSON();

  class Foo {
    #bar!: string;
    get bar() {
      return this.#bar;
    }
    set bar(value: string) {
      this.#bar = value;
    }
  }

  superjson.registerClass(Foo, {
    identifier: "Foo",
    allowProps: ["bar"],
  });

  const foo = new Foo();
  foo.bar = "foo";
  const encoded = superjson.serialize(foo);

  console.dir(encoded, { depth: null });

  console.log("will deserialize");
  const decoded = superjson.deserialize(encoded);
  console.dir(decoded, { depth: null });

Also, regardless of the stage of proposal, I simply find using new Foo() instead of Object.create(Foo.prototype) more correct.

I understand that constructor might require arguments, and it seems it prefer rather not calling constructor at all then calling it and crashing.

Both are kinda bad. Maybe registerClass could have typescript definition that requires passed class to not have constructor arguments (type NoArgsClass = new () => T;). If it has constructor arguments, TypeScript tells you not to do that and you need to define custom rule instead. But then, custom rule is not treated as 'deep', which will lead to other issues

[Skn0tt]

My understanding of # is that it'll be impossible to read or write them from the outside, so it'll never work with SuperJSON. Even if we could read the values, we can't reinstate them via the constructor, because we have no way of knowing what'd be the right constructor args. It also wouldn't be free of side effects.

Seems like we can't work around this limitation.