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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

Currently the process of updating the version of the packages is mostly manual. Before you start, first you need to make sure you have the permissions to publish the packages.

- If there is a release branch (see __Release candidate__ below) that hasn't been merged to `main` yet now is the time to do so.
- If there is a release branch (see **Release candidate** below) that hasn't been merged to `main` yet now is the time to do so.
- Make sure that you are logged in into your `npm` account. Use the command `yarn npm login` on the project folder to do this.
- While on the `main` branch.
- Perform a search an replace on all "package.json" files from the old version to the new. (ex `0.1.4` to `0.2.0`).
Expand All @@ -25,4 +25,4 @@ Currently the process of updating the version of the packages is mostly manual.
- Create an annotated git tag by running `git tag -a v0.4.0-rc.0` (replace with the version). The tag message can be the version again.
- Push commit and the tag `git push --follow-tags`.
- Publish the packages by running `yarn publish --tag rc` (it will also build the packages).
- We are currently not doing release notes for release candidates so you are all done! 🎉
- We are currently not doing release notes for release candidates so you are all done! 🎉
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"publish": "yarn workspaces foreach --topological --no-private npm publish --tolerate-republish --access public",
"test": "cross-env NODE_ENV=test jest --coverage",
"lint": "eslint .",
"types": "tsc --noEmit",
"package": "yarn workspaces foreach --topological --verbose pack --out=%s-%v.tgz && ts-node ./scripts/moveAllPackagedToArtifacts.ts"
},
"devDependencies": {
Expand Down Expand Up @@ -43,7 +44,8 @@
"jest-junit-reporter": "^1.1.0",
"jest-sonar-reporter": "2.0.0",
"prettier": "^2.3.2",
"ts-node": "^10.9.1"
"ts-node": "^10.9.1",
"typescript": "^4.8.2"
},
"packageManager": "[email protected]"
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ export function mapFacetArrayCached<M>(
facets: Facet<unknown>[],
fn: (...value: unknown[]) => M | NoValue,
equalityCheck?: EqualityCheck<M>,
onCleanup?: () => void,
): Facet<M> {
return createFacet<M>({
// pass the equalityCheck to the mapIntoObserveArray to prevent even triggering the observable
startSubscription: mapIntoObserveArray(facets, fn, equalityCheck),
startSubscription: mapIntoObserveArray(facets, fn, equalityCheck, onCleanup),
initialValue: NO_VALUE,
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export function mapFacetArrayLightweight<M>(
facets: Facet<unknown>[],
fn: (...value: unknown[]) => M | NoValue,
equalityCheck?: EqualityCheck<M>,
onCleanup?: () => void,
): Facet<M> {
return {
get: () => {
Expand All @@ -14,6 +15,6 @@ export function mapFacetArrayLightweight<M>(

return fn(...values)
},
observe: mapIntoObserveArray(facets, fn, equalityCheck),
observe: mapIntoObserveArray(facets, fn, equalityCheck, onCleanup),
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ export function mapFacetSingleCached<T, M>(
facets: Facet<T>,
fn: (value: T) => M | NoValue,
equalityCheck?: EqualityCheck<M>,
onCleanup?: () => void,
): Facet<M> {
return createFacet<M>({
// pass the equalityCheck to the mapIntoObserveSingle to prevent even triggering the observable
startSubscription: mapIntoObserveSingle(facets, fn, equalityCheck),
startSubscription: mapIntoObserveSingle(facets, fn, equalityCheck, onCleanup),
initialValue: NO_VALUE,
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export function mapFacetSingleLightweight<T, M>(
facet: Facet<T>,
fn: (value: T) => M | NoValue,
equalityCheck?: EqualityCheck<M>,
onCleanup?: () => void,
): Facet<M> {
return {
get: () => {
Expand All @@ -14,6 +15,6 @@ export function mapFacetSingleLightweight<T, M>(
return fn(value)
},

observe: mapIntoObserveSingle(facet, fn, equalityCheck),
observe: mapIntoObserveSingle(facet, fn, equalityCheck, onCleanup),
}
}
10 changes: 6 additions & 4 deletions packages/@react-facet/core/src/mapFacets/mapFacets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.

): Facet<M> {
if (facets.length === 1) {
return mapFacetSingleLightweight(facets[0], fn, equalityCheck)
return mapFacetSingleLightweight(facets[0], fn, equalityCheck, onCleanup)
} else {
return mapFacetArrayLightweight(facets, fn, equalityCheck)
return mapFacetArrayLightweight(facets, fn, equalityCheck, onCleanup)
}
}

export function mapFacetsCached<M>(
facets: Facet<unknown>[],
fn: (...value: unknown[]) => M | NoValue,
equalityCheck?: EqualityCheck<M>,
onCleanup?: () => void,
): Facet<M> {
if (facets.length === 1) {
return mapFacetSingleCached(facets[0], fn, equalityCheck)
return mapFacetSingleCached(facets[0], fn, equalityCheck, onCleanup)
} else {
return mapFacetArrayCached(facets, fn, equalityCheck)
return mapFacetArrayCached(facets, fn, equalityCheck, onCleanup)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export function mapIntoObserveArray<M>(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
fn: (...value: any[]) => M | NoValue,
equalityCheck?: EqualityCheck<M>,
onCleanup?: () => void,
): Observe<M> {
return (listener: Listener<M>) => {
let currentValue: Option<M> = NO_VALUE
Expand Down Expand Up @@ -76,6 +77,7 @@ export function mapIntoObserveArray<M>(
return () => {
cancelScheduledTask(task)
subscriptions.forEach((unsubscribe) => unsubscribe())
onCleanup?.()
}
}
}
19 changes: 16 additions & 3 deletions packages/@react-facet/core/src/mapFacets/mapIntoObserveSingle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,21 @@ export function mapIntoObserveSingle<T, M>(
facet: Facet<T>,
fn: (value: T) => M | NoValue,
equalityCheck?: EqualityCheck<M>,
onCleanup?: () => void,
): Observe<M> {
// Most common scenario is not having any equality check
if (equalityCheck === undefined) {
return (listener: Listener<M>) => {
return facet.observe((value: T) => {
const unsubscribe = facet.observe((value: T) => {
const result = fn(value)
if (result === NO_VALUE) return

listener(result)
})
return () => {
unsubscribe()
onCleanup?.()
}
}
}

Expand All @@ -23,7 +28,7 @@ export function mapIntoObserveSingle<T, M>(
return (listener: Listener<M>) => {
let currentValue: M | typeof NO_VALUE = NO_VALUE

return facet.observe((value: T) => {
const unsubscribe = facet.observe((value: T) => {
const result = fn(value)
if (result === NO_VALUE) return

Expand All @@ -44,19 +49,27 @@ export function mapIntoObserveSingle<T, M>(

listener(result)
})
return () => {
unsubscribe()
onCleanup?.()
}
}
}

// Finally we use the custom equality check
return (listener: Listener<M>) => {
const checker = equalityCheck()

return facet.observe((value: T) => {
const unsubscribe = facet.observe((value: T) => {
const result = fn(value)
if (result === NO_VALUE) return
if (checker(result)) return

listener(result)
})
return () => {
unsubscribe()
onCleanup?.()
}
}
}
4 changes: 2 additions & 2 deletions packages/@react-facet/core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ export type ExtractFacetValues<T extends ReadonlyArray<Facet<unknown>>> = {

export const FACET_FACTORY = Symbol('facet-factory')

export interface FacetFactory<T> {
(): Facet<T>
export type FacetFactory<T> = {
initializer(): Facet<T>
factory: typeof FACET_FACTORY
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import React, { ReactNode } from 'react'
import { sharedFacetsErrorBoundaryContext } from '../context/sharedFacetAvailable'
import { SharedFacetError } from '../types'

export type SharedFacetsErrorBoundaryProps = {
children: ReactNode
onError: (args: SharedFacetError) => void
}

export const SharedFacetsErrorBoundary = ({ onError, children }: SharedFacetsErrorBoundaryProps) => {
return (
<sharedFacetsErrorBoundaryContext.Provider value={onError}>{children}</sharedFacetsErrorBoundaryContext.Provider>
)
}
1 change: 1 addition & 0 deletions packages/@react-facet/shared-facet/src/components/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './SharedFacetsErrorBoundary'
13 changes: 0 additions & 13 deletions packages/@react-facet/shared-facet/src/context.ts

This file was deleted.

2 changes: 2 additions & 0 deletions packages/@react-facet/shared-facet/src/context/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from './sharedFacetAvailable'
export * from './sharedFacetDriver'
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { createContext } from 'react'
import { SharedFacetError } from '../types'

export type SharedFacetErrorBoundaryCallback = (error: SharedFacetError) => void

const noop = () => {}

export const sharedFacetsErrorBoundaryContext = createContext<SharedFacetErrorBoundaryCallback>(noop)
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import React, { ReactNode, createContext } from 'react'
import { SharedFacetDriver } from '../types'

const dummyConstructor = () => () => {}

export type SharedFacetDriverProviderProps = {
children: ReactNode
driver: SharedFacetDriver
}
export const sharedFacetDriverContext = createContext<SharedFacetDriver>(dummyConstructor)

export const SharedFacetDriverProvider = ({ children, driver }: SharedFacetDriverProviderProps) => {
return <sharedFacetDriverContext.Provider value={driver}>{children}</sharedFacetDriverContext.Provider>
}
53 changes: 53 additions & 0 deletions packages/@react-facet/shared-facet/src/functionCaching.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/* eslint-disable @typescript-eslint/no-explicit-any */

type ArgsToValueMapType<T extends Array<any>, V> = Map<T, V>

function areArraysMatchingRef<T extends Array<any>>(key1: T, key2: T): boolean {
if (key1.length !== key2.length) {
return false
}

// This assumes we will never change the order of the array key in the set, otherwise the loops will be
// exponential in relation to how many arguments are in the keys
for (let i = 0; i < key1.length; i++) {
if (key1[i] !== key2[i]) return false
}

return true
}

export function functionCaching<T extends Array<any>, V>() {
const refs: ArgsToValueMapType<T, V> = new Map()

function removeFromRef(argsKey: T) {
for (const [key] of refs) {
if (areArraysMatchingRef(key, argsKey)) {
refs.delete(key)
return
}
}
}

function getFromRef(argsKey: T): V | undefined {
for (const [key, value] of refs) {
if (areArraysMatchingRef(key, argsKey)) {
return value
}
}
}

function addToRef(argsKey: T, facet: V) {
const existingRecord = getFromRef(argsKey)
if (existingRecord) {
return
}
refs.set(argsKey, facet)
}

return {
refs,
addToRef,
removeFromRef,
getFromRef,
}
}
1 change: 1 addition & 0 deletions packages/@react-facet/shared-facet/src/hooks/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './useSharedFacet'
14 changes: 14 additions & 0 deletions packages/@react-facet/shared-facet/src/hooks/useSharedFacet.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { useContext } from 'react'
import { Facet } from '@react-facet/core'
import { sharedFacetDriverContext } from '../context/sharedFacetDriver'
import { SharedFacet, SharedFacetError } from '../types'
import { sharedFacetsErrorBoundaryContext } from '../context'

export const useSharedFacet = <T>(
sharedFacet: SharedFacet<T>,
onErrorCallback?: (error: SharedFacetError) => void,
): Facet<T> => {
const sharedFacetDriver = useContext(sharedFacetDriverContext)
const onErrorWrapperCallback = useContext(sharedFacetsErrorBoundaryContext)
return sharedFacet.initializer(sharedFacetDriver, onErrorCallback ?? onErrorWrapperCallback)
}
Loading
Loading