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

SharedUnionFields: Simplify implementation #1000

Conversation

som-sm
Copy link
Collaborator

@som-sm som-sm commented Dec 7, 2024

Simplifies the implementation of SharedUnionFields type, as mentioned here. ShareUnionFields is essentially just Pick with an extra conditional for non-recursive types.

Also, adds test cases to cover the non-recursive check, as mentioned here.

@som-sm
Copy link
Collaborator Author

som-sm commented Dec 7, 2024

@Emiyaaaaa, IMO we should skip non-recursive types even if they are mixed with recursive ones, as suggested here.

For eg., { a: 1 } | { a: 2 } | undefined currently evaluates to {} but I think should instead evaluate to { a: 1 | 2 } | undefined.

And, here's a possible implementation for the same:

type SharedUnionFields<Union> =
Exclude<Union, NonRecursiveType | ReadonlyMap<unknown, unknown> | ReadonlySet<unknown> | UnknownArray> extends infer ProcessableMembers
	?
	| Exclude<Union, ProcessableMembers>
	| (IsNever<ProcessableMembers> extends true ? never : Simplify<Pick<ProcessableMembers, keyof ProcessableMembers>>)
	: never;

WDYT?

@Emiyaaaaa
Copy link
Collaborator

And, here's a possible implementation for the same:

type SharedUnionFields<Union> =
Exclude<Union, NonRecursiveType | ReadonlyMap<unknown, unknown> | ReadonlySet<unknown> | UnknownArray> extends infer ProcessableMembers
	?
	| Exclude<Union, ProcessableMembers>
	| (IsNever<ProcessableMembers> extends true ? never : Simplify<Pick<ProcessableMembers, keyof ProcessableMembers>>)
	: never;

I think this is better

@som-sm
Copy link
Collaborator Author

som-sm commented Dec 8, 2024

@Emiyaaaaa I've updated the PR, please have a look.

I had to tweak the solution a bit from my earlier proposal to ensure it returns any for any.

test-d/shared-union-fields.ts Outdated Show resolved Hide resolved
@sindresorhus sindresorhus changed the title Simplify SharedUnionFields implementation SharedUnionFields: Simplify implementation Dec 10, 2024
@sindresorhus sindresorhus merged commit 20e71e9 into sindresorhus:main Dec 10, 2024
11 checks passed
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 this pull request may close these issues.

3 participants