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

Request: make BRAND<T> readonly to support @typescript-eslint/prefer-readonly-parameter-types #3787

Open
LumaKernel opened this issue Oct 9, 2024 · 0 comments · May be fixed by #3788
Open

Comments

@LumaKernel
Copy link

LumaKernel commented Oct 9, 2024

Currently, BRAND is defined as follows:

export type BRAND<T extends string | number | symbol> = {
  [BRAND]: { [k in T]: true };
};

https://github.com/LumaKernel/zod/blob/3032e240a0c227692bb96eedf240ed493c53f54c/src/types.ts#L4904-L4906

Here is the suggestion to update this to following readonly added type.

export type BRAND<T extends string | number | symbol> = {
  readonly [BRAND]: { readonly [k in T]: true };
};

This can support the @typescript-eslint/prefer-readonly-parameter-types rule.

This can be now confirmed as following with the repository: https://github.com/LumaKernel/zod-issue-3787

npx eslint .

/<proj>/int.ts
  8:22  error  Parameter should be a read only type  @typescript-eslint/prefer-readonly-parameter-types

/<proj>/span.ts
  15:23  error  Parameter should be a read only type  @typescript-eslint/prefer-readonly-parameter-types

✖ 2 problems (2 errors, 0 warnings)

And I already confirmed that, if I replace the BRAND definition in node_modules/ as above, this lint error is resolved for span.ts (using z.object() base).

Unfortunately, for the limitation of the eslint rule, the branded types for literal types like number & { readonly some: null } is not treated as readonly by the rule. That's why, int.ts (using z.number() base) in the repo cannot be resolved by this change, but it's still happy for the pattern of span.ts, it's mainly used in my project.

@LumaKernel LumaKernel linked a pull request Oct 9, 2024 that will close this issue
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

Successfully merging a pull request may close this issue.

1 participant