xddq / schema2typebox

Creating TypeBox code from JSON schemas
MIT License
62 stars 14 forks source link

Add $id to the generated TypeScript source #32

Closed tet-lenovo closed 8 months ago

tet-lenovo commented 10 months ago

Library version: 1.6.2

JSON schema version: draft-04, draft-06, draft-07, 2019-09, 2020-12

I am willing to contribute to fix the issue πŸ’š

The current behavior

Currently, TypeBox source is generated that looks something like this:

const obj1 = Type.Object({
    prop1: Type.String(),
    prop2: Type.String()
});

The expected behavior

I would like it to include a top-level $id for everything generated:

const obj1 = Type.Object({
    prop1: Type.String(),
    prop2: Type.String()
}, {
    $id: "Obj1Id"
});

So I can reference it like this:

const obj2 = Type.Object({
    obj1: Type.Ref(obj1),
    other: Type.String()
}, {
    $id: "Obj2Id"
});

Why does it happen? What/How to fix the issue?

I did hack together a quick "post processing" type change in the schema2typebox() function that does what I want. Not sure if this is the best way to do it or not but it at least demonstrates what I am asking for.

...
    const exportedName = createExportNameForSchema(dereferencedSchema);
    const exportedType = createExportedTypeForName(exportedName);
    const start = typeBoxType.slice(0, typeBoxType.lastIndexOf(");"));
    const end = typeBoxType.slice(typeBoxType.lastIndexOf(");"));
    typeBoxType = `${start}, { $id: "${exportedName}" }${end}`;
    return `${createImportStatements()}

${typeBoxType.includes("OneOf([") ? (0, exports.createOneOfTypeboxSupportCode)() : ""}
${exportedType}
export const ${exportedName} = ${typeBoxType}`;
xddq commented 10 months ago

Hey!

Seems to be fine. Probably name it something like addJsonSchemaIdToTypeboxType which takes the typeBoxType and returns the code.

If you want, you are welcome to create a PR for this! Could also consider adding a new simple test with using Refs for this.

tet-lenovo commented 10 months ago

I started using this approach to add $id and broke the "schema.org - dayOfWeek" test since I just blindly insert $id based on ");". I think there are two options: 1) I can check the code for "$id" and not insert one, 2) I can change collect() so that it passes the exportedName down to all of the parsers and inserts $id appropriately. With option 1, it can still fail if the string "$id" is in any of the JSON text plus it only would work because that test happens to already include $id in the JSON schema. Option 2 is probably the better way but more invasive. Looking for some direction on the preferred approach.

xddq commented 10 months ago

Oh.. I see. If I am seeing this correctky I guess it is actually best to add the id right after we dereference the source schema here into the object and let collect() do its work!

Basically, if dereferencedschema.$id is undefined, you can add exportedName (or whatever you think is best) to the id field. Otherwise do nothing with it.

Can you check this and perhaps send a PR if it works out?

tet-lenovo commented 10 months ago

So, more roadblocks and I am not sure I totally understand why. Some but not all of the parse functions use parseSchemaOptions() and generate the proper TypeScript if I just insert dereferencedSchema.$id. Once I made some of those changes, prettier seems like it is formatting the code differently for unit tests vs. the generated code. I don't really know anything about prettier so I can't find a way to make the code formats match and satisfy the unit tests. Maybe there was some reason I don't understand that the code never fully supported parseSchemaOptions()?

Below is really the main part of what I was trying to do. The rest is all changes to make collect() work consistently and then fixes to the unit tests to deal with the schema options added to the TypeScript code. I had some down time this week but I have used it all up trying to get this working so I don't think I can spend any more time trying to get it to work. Since you hopefully understand the code better, any chance you can add support for this? The big issue seems to be fixing the parse functions so they all deal with schema options consistently.

  // If this is a JSON schema without a $id, set it to the exported name.
  if (!(typeof dereferencedSchema === "boolean") && !dereferencedSchema.$id) {
    dereferencedSchema.$id = exportedName;
  }
xddq commented 10 months ago

Alright, still thanks for the effort! Will have a look in the coming weeks.

xddq commented 8 months ago

done in https://github.com/xddq/schema2typebox/pull/33

If you have issues please create a new one