Fix IdSchema and PathSchema types

[solimant]

Reasons for making this change

The way IdSchema and PathSchema are defined results in reaching a conflicting type when the type argument passed to those types is of a primitive type, such as string.

For example, in the snippet below (TS playground link), when the type argument for IdSchema is string (i.e. IdSchema<string>), the effective resolved type would be an intersection of FieldId and string, which is impossible:

type FieldId = {
  $id: string;
};

type IdSchema<T = any> = FieldId & {
  [key in keyof T]?: IdSchema<T[key]>;
};

type FieldPath = {
  $name: string;
};

type PathSchema<T = any> = FieldPath & {
  [key in keyof T]?: PathSchema<T[key]>;
};

type StringIdSchema = IdSchema<string>; // i.e. FieldId & string

type StringPathSchema = PathSchema<string>; // i.e. FieldPath & string

// TypeScript error:
// Type '{ $id: string; }' is not assignable to type 'StringIdSchema'.
//  Type '{ $id: string; }' is not assignable to type 'string'.
const idSchema: StringIdSchema = {$id: 'some_id'};

// TypeScript error:
// Type '{ $name: string; }' is not assignable to type 'StringPathSchema'.
//  Type '{ $name: string; }' is not assignable to type 'string'.
const pathSchema: StringPathSchema = {$name: 'some_path'};

When the type argument is string, keyof string in TypeScript actually results in keyof String, which includes index signatures like length, toString, etc. However, because IdSchema and PathSchema map over the keys of the string primitive type, we need to consider that string itself is not treated as an object with properties that can be indexed. Thus, IdSchema and PathSchema will effectively ignore any keys and result in string.

This PR addresses this contradiction by limiting the resolved type of IdSchema and PathSchema to just FieldId and FieldPath, respectively, when the type argument is a primitive type, in which it leverages the existing type GenericObjectType. This would let TypeScript accept the types as intended (TS playground link):

type GenericObjectType = {
    [name: string]: any;
};

type FieldId = {
  $id: string;
};

type IdSchema<T = any> = T extends GenericObjectType
  ? FieldId & {
      [key in keyof T]?: IdSchema<T[key]>;
    }
  : FieldId;

type FieldPath = {
  $name: string;
};

type PathSchema<T = any> = T extends GenericObjectType
  ? FieldPath & {
      [key in keyof T]?: PathSchema<T[key]>;
    }
  : FieldPath;

type StringIdSchema = IdSchema<string>; // i.e. FieldId

type StringPathSchema = PathSchema<string>; // i.e. FieldPath

type FooType = {foo: string};

type FooIdSchema = IdSchema<FooType>; // i.e. FieldId & { foo?: FieldId | undefined };

// No TypeScript error
const idSchema: StringIdSchema = {$id: 'some_id'};

// No TypeScript error
const pathSchema: StringPathSchema = {$name: 'some_path'};

// No TypeScript error
const fooIdSchema: FooIdSchema = {$id: 'some_id', foo: {$id: 'some_foo_id'}};

If this is related to existing tickets, include links to them as well. Use the syntax fixes #[issue number] (ex: fixes #123).

If your PR is non-trivial and you'd like to schedule a synchronous review, please add it to the weekly meeting agenda: https://docs.google.com/document/d/12PjTvv21k6LIky6bNQVnsplMLLnmEuypTLQF8a-8Wss/edit

Checklist

• I'm updating documentation
  • I've checked the rendering of the Markdown text I've added
• I'm adding or updating code
  • I've added and/or updated tests. I've run npm run test:update to update snapshots, if needed.
  • I've updated docs if needed
  • I've updated the changelog with a description of the PR
• I'm adding a new feature
  • I've updated the playground with an example use of the feature

[heath-freenome]

@solimant It looks like your type updates broke the building of utils. Can you look into this?

[heath-freenome]

@solimant Have you figured out how to fix the build issues?

[solimant]

Hi @heath-freenome - I was actually looking into it today. It seems to have to do with how toIdSchemaInternal is using keys from the given schema param. I plan to share an update today or tomorrow.

[solimant]

@heath-freenome - apologies for the delay. Got this fixed on my local. Will wait to see if tests pass.

[solimant]

@heath-freenome - could get the tests running when you get a chance? Thank you 🙏🏼

[solimant]

@heath-freenome - looks like CI failed again. I had run npm run build, and it passed locally. Are there other CI builds I should run locally?

EDIT:
My bad. I had thought that npm run build passed, but maybe my branch was stale. Looking into it.

[solimant]

@heath-freenome - looks like tests passed this time. The PR branch is out of date, though, which I'm happy to rebase. Let me know how you'd like me to proceed.

[solimant]

@heath-freenome - are you good merging this? What are next steps?

[heath-freenome]

@solimant why remove the generic? Did this cause problems?

[heath-freenome]

@solimant why remove the generic? Did this cause problems?

[heath-freenome]

Does it work to update the pathSchema type above? Right now it is (incorrectly?) defined as PathSchema. Maybe it wants to be PathSchema<T> . Or maybe create a local variable that is cast once to the proper type? I.e.

// on line 74 do this?
const keyablePathSchema = pathSchema as { [key in keyof PathSchema<T>]: PathSchema<T> });

This avoids multiple casts later and can be commented once as to why its being done

And then this line and all others below become:

keyablePathSchema[i as keyof PathSchema<T>] = ...

[heath-freenome]

Add a comment here to explain why the cast

[heath-freenome]

@heath-freenome - are you good merging this? What are next steps?

@solimant sorry, I was waiting to review this with @nickgros and we had some additional feedback

[solimant]

Thank you for the review and feedback, @heath-freenome and @nickgros. I'll work on incorporating that.