sinclairzx81 / typebox

Json Schema Type Builder with Static Type Resolution for TypeScript
Other
5.08k stars 161 forks source link

Fix records in modules #1076

Closed zoriya closed 1 week ago

zoriya commented 1 week ago

Without this, all record in modules were inferred as never.

sinclairzx81 commented 1 week ago

@zoriya Hi, thanks for this PR (and great work navigating this aspect of the code!)

At the moment, there is actually quite a lot of Module related work being done in the ref branch. Would it be possible to retarget your PR for ref instead of master ? Just note that ref branch moves the infer logic from module.ts, to src/type/module/infer.ts. This code will be going out on 0.34.1.


A lot of the work on the ref branch relates to mutual recursive / auto deref / computed types, this to enable the following (which isn't possible on 0.34.0)

const Module = Type.Module({
   T: Type.Object({ x: Type.Number() }),
   B: Type.Partial(Type.Ref('T')) // where interior Ref is dereferenced via Import
})

Record actually fits into the computed type category and is yet to be worked on, so this PR would certainly help make a start with the inference side of things. Actually any help would be appreciated with the new Module feature! :)

Thanks again, Keep me posted. S

sinclairzx81 commented 1 week ago

@zoriya Hey, just an update here.

It might be good to hold off retargeting to ref for the time being. I'm currently in the process of working through some fairly difficult challenges supporting deferred computed types, and things are a bit influx atm. There is a high likelihood of things changing quite a bit internally (inclusive of potentially implementing functions for each of the TInferX types (which would include Record also)). Some of the code around this is very intricate and spans a lot of library, so I'll need to be across most of it.

I'll keep this PR here for the time being, but will need to defer merging through while I work through some of internals. Will keep you posted S

sinclairzx81 commented 1 week ago

@zoriya Hiya!

Ended up having to implement some fairly sophisticated mapping for Record and other computable types. There is still quite a lot to do, but the implementation turned out to be less than trivial :( Can take a look at the updates below if you're interested.

https://github.com/sinclairzx81/typebox/blob/master/src/type/module/compute.ts#L189-L220

// ------------------------------------------------------------------
// Computed
// ------------------------------------------------------------------
// prettier-ignore
type TFromComputed<ModuleProperties extends TProperties, Target extends string, Parameters extends TSchema[],
  Dereferenced extends TSchema[] = TDerefParameters<ModuleProperties, Parameters>
> = (
  Target extends 'Awaited' ? TFromAwaited<Dereferenced> :
  Target extends 'Index' ? TFromIndex<Dereferenced> :
  Target extends 'KeyOf' ? TFromKeyOf<Dereferenced> :
  Target extends 'Partial' ? TFromPartial<Dereferenced> :
  Target extends 'Omit' ? TFromOmit<Dereferenced> :
  Target extends 'Pick' ? TFromPick<Dereferenced> :
  Target extends 'Record' ? TFromRecord<Dereferenced> :
  Target extends 'Required' ? TFromRequired<Dereferenced> :
  TNever
)
// prettier-ignore
function FromComputed<ModuleProperties extends TProperties, Target extends string, Parameters extends TSchema[]>(moduleProperties: ModuleProperties, target: Target, parameters: Parameters): TFromComputed<ModuleProperties, Target, Parameters> {
  const dereferenced = DerefParameters(moduleProperties, parameters)
  return (
    target === 'Awaited' ? FromAwaited(dereferenced) :
    target === 'Index' ? FromIndex(dereferenced) :
    target === 'KeyOf' ? FromKeyOf(dereferenced) :
    target === 'Partial' ? FromPartial(dereferenced) :
    target === 'Omit' ? FromOmit(dereferenced) :
    target === 'Pick' ? FromPick(dereferenced) :
    target === 'Record' ? FromRecord(dereferenced) :
    target === 'Required' ? FromRequired(dereferenced) :
    Never()
  ) as never
}

I really appreciate the PR tho (and the time taken to go through the type mapping code). It's actually good to have people dig into the type mapping and submit fixes (there's a lot of it!). Will close out this one for now though as the Record got implemented via the new Computed Type infrastructure.

Thanks again, All the best! S

zoriya commented 1 week ago

Thanks for keeping me updated!

Types looked surprisingly readable for the typescript shenanigans that were done, typebox is really cool!