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

Type Inference Improvement Ideas #7

Open
ceigey opened this issue Dec 29, 2024 · 1 comment
Open

Type Inference Improvement Ideas #7

ceigey opened this issue Dec 29, 2024 · 1 comment

Comments

@ceigey
Copy link

ceigey commented Dec 29, 2024

Hi there,

I've been playing around with jam:easy-schema and jam:methods in a project installed as a git submodule in the packages folder instead of the traditional way which let me play around a bit with the easy-schema.d.ts file.

I noticed some potential improvements to type inference could be made, but I'm not sure how much this affects performance or has any limitations for other users. I had to do a lot of reloading in the editor to see the improvements each time, which takes "the usual 2 seconds or so" for TS to start linting.

Anyway, I need to clean up the changes in a new branch, match the code style etc, and make a pull request... but the summary is basically as follows.

1. Extend the types provided by Meteor Check:

// easy-schema.d.ts
import { Match } from 'meteor/check'

export declare const has: unique symbol;

type HasFluentSchema<T> = { [has]: BaseSchema<T> }

// Object pattern limited to 
export type ObjectPattern<T> = { type: HasFluentSchema<T> }

export type Pattern = 
  | BaseSchema<any>
  | HasFluentSchema<any>
  | ObjectPattern<any>
  | [Pattern]
  // Meteor Check doesn't require Pattern[], because check schemas are ephemeral
  // so typescript gives e.g. [String] the strict type of [StringConstructor]
  // but a schema declared as a variable does not have this benefit of the doubt
  // and is inferred as StringConstructor[]
  // Object.freeze or `as const` makes TS complain about mutability... can't win.
  | Pattern[]
  | {[key: string]: Pattern}
  | Match.Pattern

export type PatternMatch<T extends Pattern> =
  T extends BaseSchema<infer U> ? U :
  T extends HasFluentSchema<infer U> ? U :
  T extends [Pattern] ? PatternMatch<T[0]>[] :
  T extends Pattern[] ? PatternMatch<T[0]>[] :
  T extends ObjectPattern<infer U> ? U :
  T extends {[key: string]: Pattern} ? {[K in keyof T]: PatternMatch<T[K]>} :
  Match.PatternMatch<T>

2. Everything that uses Match.Matcher needs its signature updated

The gist is that Match.Matcher stays, but Match.Pattern or Match.PatternMatch need to be replaced with the local version instead.

Actually I found an issue with the types in main's HEAD, they were not namespaced properly in some cases so inference was broken originally because of that, but I really wanted to get all the extra schema definition patterns working too so I've gone far past that point

- export declare function Optional<T extends Match.Pattern>(
+ export declare function Optional<T extends Pattern>(
  pattern: T
- ): Match.Matcher<Match.PatternMatch<T> | undefined | null>;
+ ): Match.Matcher<PatternMatch<T> | undefined | null>;

3. Modify attachSchema to return its own instance, and add assertion return type

declare module 'meteor/mongo' {
  module Mongo {
    // If you're brave you can modify the signature so it accepts an additional type param
    // representing the schema's pattern, but seems pretty messy
    // also this might affect the `transform` method's type safety.
    interface Collection<T> {
      attachSchema<U extends Pattern>(schema: U): Collection<PatternMatch<U>>
      // Also make sure this points to the right pattern
      schema?: Pattern;
    }
  }
}

Then you can do e.g.:

export const coll = new Mongo.Collection('things').attachSchema({
    _id: String,
    name: String[has].min(1).max(10),
    age: Optional(Number),
    uri: { type: String, max: 10 },
    codes: [String],
    dob: [{ type: Date }],
})

And you should get something inferred like:

// TS will complain about `name` and `dob`
const v = await coll.findOneAsync({ name: 10, dob: 'hi' })

// v's inferred type is:
const v: {
    _id: string;
    name: string;
    age: number | null | undefined;
    testing: string;
    uri: string;
    codes: string[];
    dob: Date[];
} | undefined

Let me know how this sounds, I won't have time to prepare a PR straight away so I can come back and do that. But maybe I'm working in the wrong direction to what you want for the library, e.g. if you wanted to add Zod support or something which might make this a little redundant.

@jamauro
Copy link
Owner

jamauro commented Dec 29, 2024

Hey, sounds great. I'm all for improvements that make the DX better. I wouldn't expect any performance impacts since the changes seem isolated to the .d.ts but let me know if I'm missing something.

I don't plan to support Zod with this package so I'm sure other TS users will welcome your suggested changes.

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