flightcontrolhq / superjson

Safely serialize JavaScript expressions to a superset of JSON, which includes Dates, BigInts, and more.
https://www.flightcontrol.dev?ref=superjson
MIT License
4.13k stars 91 forks source link

Class object is not supported? #227

Closed panmenghan closed 1 year ago

panmenghan commented 1 year ago
import {serialize} from 'superjson'

class A {
  constructor() {
    this.d = new Date()
  }
}

const a = new A()

console.log(serialize(a))
// -> { json: A { d: 2023-03-20T02:17:43.848Z } }
// why no meta? 

console.log(serialize({...a}))
// -> {
//   json: { d: '2023-03-20T02:17:43.848Z' },
//   meta: { values: { d: [Array] } }
// }
sky-code commented 1 year ago

Got the same issue, I used structuredClone(serialize(data)) as work around.

Skn0tt commented 1 year ago

Hi! You can use .registerClass for this. It's not documented, sadly, but there's a unit test that should explain it: https://github.com/blitz-js/superjson/blob/9d81c754ad64492499f4cf24371fc2fde5f99d96/src/index.test.ts#L784

A PR with a doc improvement would be very welcome :)

woody146 commented 1 year ago

Hi! You can use .registerClass for this. It's not documented, sadly, but there's a unit test that should explain it:

https://github.com/blitz-js/superjson/blob/9d81c754ad64492499f4cf24371fc2fde5f99d96/src/index.test.ts#L784

A PR with a doc improvement would be very welcome :)

registerClass isn't good choice if there're many classes

Skn0tt commented 1 year ago

registerClass isn't good choice if there're many classes

Could you elaborate on your reasoning for this? If there's something missing for usecases with a lot of classes, I'm happy to add that functionality to SuperJSON!

woody146 commented 1 year ago

registerClass isn't good choice if there're many classes

Could you elaborate on your reasoning for this? If there's something missing for usecases with a lot of classes, I'm happy to add that functionality to SuperJSON!

I use Typeorm to define tables in database. In API response, i return instance of entity class. If i have 100 tables in database, i must register 100 classes. So it's inconvenient

Skn0tt commented 1 year ago

Interesting. Are you using SuperJSON for serialization between client and server, or for something else? If yes, what's your usecase for using TypeORM class feature from the client?

woody146 commented 1 year ago

No, I use SuperJSON to parse datetime field in entity for client.

@Entity()
export class User {
    @PrimaryGeneratedColumn()
    id: number

    @Column()
    name: string

    @CreateDateColumn()
    createdDate: Date
}

function getUserApi(userId) {
  return dbSession.getRepository(User).find({
    where: {
        id: userId
    },
  });
}

const user = await getUserApi(1);
SuperJSON.serialize(user)  // there's no meta
sky-code commented 1 year ago

I got exactly the same issue when I used superjson with trpc and typeorm

Skn0tt commented 1 year ago

That makes sense, thank you! One solution you could think about is to write your own decorator (similar to @Entity) that wraps the SuperJSON.registerClass call:

function RegisterSuperJSON(klass) {
   SuperJSON.registerClass(klass)
   return klass
}

// ...

@RegisterSuperJSON
@Entity
class User { ... }
markedwards commented 6 months ago

Hitting this too. I tried to work around it by calling superjson.register() on a common base class. That doesn't work, however. Is it not possible to support this? Why can't subclasses be registered all at once via their base class?

Skn0tt commented 6 months ago

It's impossible to register all subclasses via their base class, because SuperJSON needs to have a reference to the exact subclass to be able to reinstantiate an object with that prototype. If we didn't have it, we'd turn all instances of class Foo extends Bar into instances of Bar when recreating them, which isn't correct behaviour.

markedwards commented 6 months ago

Yeah, I sort of figured that out after my post above, but thanks for the response.

It might be helpful to have the option to attempt to force certain classes to be serialized as plain objects. For example in the TypeORM case, the object happen to be class instances, but they can easily be serialized to plain objects, and for some workflows you need that conversion for downstream compatibility. Is there a way make that happen during superjson.stringify()?

My current solution is to pre-process with a recursive plain() step prior to superjson.stringify(). Perhaps this problem is simply out of scope for superjson.

Skn0tt commented 6 months ago

Generally speaking, I think this problem is out of scope, yes. You should be able to make this work using .registerCustom, though, like this:

SuperJSON.registerCustom(
  {
    isApplicable: (v): v is typeorm object => ...,
    serialize: v => plain(v),
    deserialize: v => v,
  },
  'typeorm object'
);
markedwards commented 6 months ago

I experimented with that solution. The problem is I want superjson to handle the serialization and manage things like Date, I just need to convert the object to plain so superjson will accept it. If I use the registerCustom approach, Date ends up as string, for example. So what I really need here is a way to preprocess custom types.

Skn0tt commented 6 months ago

Gotcha. Yeah, that will be hard to do with SuperJSON, sorry.

markedwards commented 6 months ago

No problem. It's not difficult to implement a simple preprocessor function for this. Slightly less efficient because we have to recurse the object structure twice. But it's not a big problem.