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

Support for Go To Definition, Rename Symbol and other IDE features #1115

Closed
bentefay opened this issue May 4, 2022 · 3 comments
Closed

Support for Go To Definition, Rename Symbol and other IDE features #1115

bentefay opened this issue May 4, 2022 · 3 comments

Comments

@bentefay
Copy link
Contributor

bentefay commented May 4, 2022

Thanks for your amazing work on this library! I'm a big fan of the approach you have taken with Zod of defining a type's schema only once rather than duplicatively for each type and its parser.

The one major downside I have found with using Zod for declaring types is that Visual Studio Code (and I assume the TypeScript language server more generally) does not seem to be able to connect usages of object properties back to their declaration in z.object.

For example, consider the following code:

const Example = z.object({ property: z.string() });

type Example = z.infer<typeof Example>;

const instance: Example = { property: "value" };

Using any language server action like Go To Definition, Find Usages or Rename Symbol on Example["property"] does not work. This is a particular pain point for Rename Symbol, which is incredibly useful when working on a large code base.

I assume this is a limitation of the Typescript language server given the specific type-level programming that Zod is using. However, these features do appear to work with io-ts (a library that takes a similar approach), which makes me think it may be possible to implement Zod's type mappings in a way that preserves these IDE features, at least for some scenarios.

Is this a valuable goal to pursue, or have I misunderstood how most people are using Zod?

Digging a little deeper, TypeScript does not appear to be able to connect the properties in an object to those same properties in its associated mapped type if the keys are changed. This appears to be what is causing the IDE features to not work. For example, consider a simplified version of the return type of objectType in Zod:

type optionalKeys<T extends object> = {
    [k in keyof T]: k;
}[keyof T];

type requiredKeys<T extends object> = {
    [k in keyof T]: undefined extends T[k] ? never : k;
}[keyof T];

export type addQuestionMarks<T extends object> = {
    [k in optionalKeys<T>]?: T[k];
} & {
    [k in requiredKeys<T>]: T[k];
};

export type mergeIntersections<T extends object> = { [K in keyof T]: T[K] };

type Example2 = { property: string; optionalProperty: string | undefined };

const instance2: mergeIntersections<addQuestionMarks<Example2>> = { property: "" };

// type Instance2 = {
//    property: string;
//    optionalProperty?: string | undefined;
// }
type Instance2 = typeof instance2;

This code is responsible for making schemas marked with .optional() optional (rather than just undefined) in the final object type. The addQuestionMarks type is transforming the keys in T using optionalKeys and requiredKeys. Due to these transformations of the keys of T, TypeScript can no longer connect the original type's keys to the mapped type's keys. As a result, using Go To Definition on property on the line starting with const instance2 does not work.

However, I believe we can get the same outcome in a way that preserves the IDE features:

export type addQuestionMarksPreserve<T extends object> = {
    [k in keyof T]?: T[k];
} & {
    [k in requiredKeys<T>]: T[k];
};

type Example3 = { property: string; optionalProperty: string | undefined };

const instance3: mergeIntersections<addQuestionMarksPreserve<Example3>> = { property: "" };

// type Instance3 = {
//    property: string;
//    optionalProperty?: string | undefined;
// }
type Instance3 = typeof instance3;

All I have done is changed [k in optionalKeys<T>]?: T[k]; to [k in keyof T]?: T[k];. Using Go To Definition and Rename Symbol on property on the line starting with const instance3 now works as you would expect.
I believe this works because { [k in keyof T]?: T[k]; } directly passes on the keys of T without transforming them. This allows TypeScript to connect the original type with the mapped type. We then rely on the fact that { a?: A | undefined } & { a: A } merges to { a: A }, giving us the original type mapping behaviour.

What do you think? Is it worth seeing whether these sorts of changes can improve the IDE experience when using Zod?

@scotttrinh
Copy link
Collaborator

I don't have any first hand knowledge of what might be going on here, but I think if we can make a backwards compatible change that preserves our existing behavior and doesn't add much maintenance or cognitive overhead, I'm absolutely thrilled to help get a PR merged that improves editor ergonomics.

I think the one thing I would want to see in a PR that updates this type is some additional type-level tests to ensure we're not breaking anything in the ecosystem, so feel free to open a PR with these changes and we can get that going together.

@bentefay
Copy link
Contributor Author

bentefay commented May 5, 2022

Thanks for the quick response @scotttrinh!

I've opened a PR: #1117

@colinhacks
Copy link
Owner

Merged! This is fantastic stuff, thank you 👏

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

3 participants