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

Describing and handling sharedFacet errors #125

Closed
wants to merge 16 commits into from
Closed

Conversation

Shenato
Copy link
Contributor

@Shenato Shenato commented Sep 1, 2023

This PR adds a way to define errors for SharedFacets in the SharedFacetDriver using the onError callback like so

const sharedFacetDriver: SharedFacetDriver = (name, onChange, onError) => {
  if (name === 'bar') {
    onError?.({ facetName: 'bar', facetError: 'facetUnavailable' })
    return () => {}
  }
  onChange({ bar: 'testing 123', values: ['a', 'b', 'c'] })
  return () => {}
}

The sharedFacetDriver defines an error case, which calls an onError handler which should be set by either a specific useSharedFacet() instance or one of the new <SharedFacetsErrorBoundary /> instances which should catch any errors in any facet within it's scope and handle the error with the error handler you pass to it

Example usage of a local facet error handler

useSharedFacet(barFacet, (error) => {
  doSomethingWithError(error)
  maybeUnmountMyFailingComponent(false)
}),

Example of the usage of a <SharedFacetsErrorBoundary />

const [mount, setMount] = useFacetState(true)
return (
  <SharedFacetsErrorBoundary
    onError={(error) => {
      doSomethingElseWithError(error)
      unmountMyFailingScreen(false)
    }}
  >
    <Mount when={mount}>{children}</Mount>
  </SharedFacetsErrorBoundary>
)

The error you get passed to your callback from the sharedFacetDriver is of type SharedFacetError and it's basic type looks like so

export interface SharedFacetError {
  facetName: string
  message?: string
}

And the the end user can re-define what the error type should look like using the declaration merging typescript feature, we went with this implementation to make sure the end user gets typesafety in their error handling which they can re-define to suit their needs.

// This is how we've opted to allow users to extend the error type to their usecases, they re-declare the
// interface for SharedErrorFacet with whatever extensions they want
// the original type just has facetName as mandatory and we add ontop of that facetError as mandatory in this case
declare module '@mojang/shared-facet' {
  export interface SharedFacetError {
    facetError: string
  }
}

The end result of the type in the user's codebase would look something like this.

export interface SharedFacetError {
  facetName: string
  message?: string
  facetError: string
}

@@ -8,22 +8,24 @@ export function mapFacetsLightweight<M>(
facets: Facet<unknown>[],
fn: (...value: unknown[]) => M | NoValue,
equalityCheck?: EqualityCheck<M>,
onCleanup?: () => void,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this was added at one point, but its no longer needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of this needed change to cleanup the new cache for SharedFacets in useSharedSelector I had to change alll functions similar to mapFacetsCached to include an optional onCleanup callback function to allow the cleaning up the cache when the facets are no longer there.

@Shenato
Copy link
Contributor Author

Shenato commented Feb 8, 2024

A new facet API might be introduced soon which will render this PR obselete, consider taking up the effort again after such change is done

@Shenato Shenato closed this Feb 8, 2024
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