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

Update the useSharedFacet typing #144

Closed
wants to merge 1 commit into from
Closed

Conversation

viccon
Copy link
Collaborator

@viccon viccon commented Mar 7, 2025

Typescript enforces invariance in generics which means that the useSharedFacet requires a single specific type. This leads to type errors when trying to pass a union, e.g:

const sharedFacet = getFacet(worldMode) // SharedFacet<CreateNewWorldFacet> | SharedFacet<WorldEditorFacet> | SharedFacet<RealmWorldEditorFacet>
const regularFacetUnion = useSharedFacet(sharedFacetUnion) // This errors because the types are not assignable to each other.

However, with the changes I'm introducing here the hook now leverages conditional types with infer to automatically distribute over unions. So when a union of shared facets is passed to useSharedFacet, the return type becomes a union of the corresponding facet types. Which means that the regularFacetUnion above will be of type Facet<CreateNewWorldFacet> | Facet<WorldEditorFacet> | Facet<RealmWorldEditorFacet>.

We are then able to use that facet union with our other hooks like useFacetMap:

const f = useFacetMap(
	(f) => {
		if ('worldData' in f) {
			return f.worldData
		}
		if ('realmWorldData' in f) {
			return f.realmWorldData
		}
	},
	[],
	[regularFacetUnion],
)

One downside is that the updated implementation now uses a type assertion (as InferFacet<T>) because typescript cannot automatically verify that the value returned by sharedFacet(driver) conforms to the conditional type. However, I believe that this cast is always safe based on the functions behaviour.

@viccon viccon closed this Mar 11, 2025
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.

1 participant