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

bug: inconsistent field ordering in serialized object when using createModelSchema with inheritance #179

Open
akphi opened this issue Apr 25, 2023 · 1 comment

Comments

@akphi
Copy link
Contributor

akphi commented Apr 25, 2023

Description

When I specify the model schema for 2 classes where one extends the other, the order of fields during serialization of the subclass sometimes do not follow the order specified in in createModelSchema. Look at the example I provided

import { createModelSchema, primitive, serialize } from "serializr";

class Person {
  name;
}

class Worker extends Person {
  company;
}

// 1.
// const workerModelSchema = createModelSchema(Worker, {
//   company: primitive(),
//   name: primitive(),
// });
//
// output: { company: 'FirmX', name: 'John' } --> this works

const personModelSchema = createModelSchema(Person, {
  name: primitive(),
});

// 2.
const workerModelSchema = createModelSchema(Worker, {
  company: primitive(),
  name: primitive(),
});
//
// output: { name: 'John', company: 'FirmX' } --> this doesn't work as expected

// 3.
// const workerModelSchema2 = {
//   factory: () => new Worker(),
//   props: {
//     company: primitive(),
//     name: primitive(),
//   },
// };
//
// output: { company: 'FirmX', name: 'John' } --> this works

const newWorker = new Worker();
newWorker.name = "John";
newWorker.company = "FirmX";

const json = serialize(workerModelSchema, newWorker);

console.log(json);

I have 2 classes Person and Worker extends Person. When using createModelSchema, if I specify the model schema for Person before I specify the model schema for Worker, when I try to serialize an instance of Worker, the props in the model schema of Worker which overlap with the props in the model schema of Person will always be arranged first, and the fields which specialized to Worker will be arranged last, in the example, I'm trying to serialize using workerModelSchema, which specifies company field to go before name, but in the output, I don't see this.

Now, if I use the (1), where I specify the model schema of Worker before I do Person, I would get what I expected, or if I do (3) where I avoid using createModelSchema, I would get what I expected as well.

I really think this is a bug due to the inconsistency we see here, I suppose this has to do with some intricate handling we do in createModelSchema

Reproduce

akphi/issue-repo#13

Thoughts & Ideas

I think it comes down to this logic here

if (schema.extends) res = serializeWithSchema(schema.extends, obj);

My workaround right now would just be to do something like

const workerSchema = createModelSchema(...);
workerSchema.extends = undefined;

But it's kind of hacky, I wonder if there's a way we can just default to have createModelSchema not doing magic, or maybe have a flag on createModelSchema to turn off this auto-detect extends mechanism, or maybe a flag on serialize to disable this, or a global configuration for serializr to disable this behavior. Let me know what you guys think! Thanks

@akphi akphi changed the title Inconsistent field ordering in serialized object when using createModelSchema with inheritance bug: inconsistent field ordering in serialized object when using createModelSchema with inheritance Apr 25, 2023
@paulftw
Copy link

paulftw commented Nov 29, 2023

Based on this example I think the right way to serialize subclasses is to remove name from Worker and add getDefaultModelSchema(Worker).extends = getDefaultModelSchema(Person).
This way name will always go first as the base class state should be (de)serialized before subclasses.

In general, having two serializers for name is a dangerous route because it's error prone: implementation details leak into sub class, have to remember to update multiple code locations if name logic changes, it's not clear what a parent should do when two subclasses serialize name in different ways, etc.

Currently all 3 snippets seem to declare inheritance in JS classes but instead of relying on the serializr inheritance features they use various workarounds to get a (sort of) correct output.

Worth mentioning that while JS guarantees key order, JSON dictionaries are still unsorted dictionaries and it's better to avoid relying on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants