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 #145

Merged
merged 2 commits into from
Mar 12, 2025
Merged

Update the useSharedFacet typing #145

merged 2 commits into from
Mar 12, 2025

Conversation

viccon
Copy link
Collaborator

@viccon viccon commented Mar 11, 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 merged commit c0693d7 into main Mar 12, 2025
12 checks passed
@viccon viccon deleted the v0.6.2 branch March 12, 2025 10:38
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.

2 participants