From 65f331e67081230af4731d3daf74f8c5ebd2ff2f Mon Sep 17 00:00:00 2001 From: Omar ElGaml Date: Wed, 24 Aug 2022 16:11:49 +0200 Subject: [PATCH 1/9] Separated logic to create fake facet and createFaceContext --- .../core/src/createFacetContext.tsx | 13 ++----------- .../core/src/facet/createFakeFacet.ts | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 11 deletions(-) create mode 100644 packages/@react-facet/core/src/facet/createFakeFacet.ts diff --git a/packages/@react-facet/core/src/createFacetContext.tsx b/packages/@react-facet/core/src/createFacetContext.tsx index 5c4fa8f8..8580fa35 100644 --- a/packages/@react-facet/core/src/createFacetContext.tsx +++ b/packages/@react-facet/core/src/createFacetContext.tsx @@ -1,17 +1,8 @@ +import { createFakeFacet } from './facet/createFakeFacet' import { createContext } from 'react' -import { Facet } from './types' export function createFacetContext(initialValue: T) { - const facet: Facet = { - get: () => initialValue, - observe: (listener) => { - if (process.env.NODE_ENV !== 'production') { - console.log('Missing Provider') - } - listener(initialValue) - return () => {} - }, - } + const facet = createFakeFacet(initialValue) const context = createContext(facet) return context } diff --git a/packages/@react-facet/core/src/facet/createFakeFacet.ts b/packages/@react-facet/core/src/facet/createFakeFacet.ts new file mode 100644 index 00000000..0a121901 --- /dev/null +++ b/packages/@react-facet/core/src/facet/createFakeFacet.ts @@ -0,0 +1,17 @@ +import { Facet } from '..' +/** + * Creates a nonwritable barebones facet to be used when you need an initial facet value that's meant to be replaced later by a real facet. Ex: with `useContext()` + */ +export function createFakeFacet(value: T): Facet { + const facet: Facet = { + get: () => value, + observe: (listener) => { + if (process.env.NODE_ENV !== 'production') { + console.log(`Accessing a fake facet, perhaps you're missing a Context Provider?`) + } + listener(value) + return () => {} + }, + } + return facet +} From f5b45cc1a9646100cb328a986a6945716323fad3 Mon Sep 17 00:00:00 2001 From: Omar ElGaml Date: Thu, 25 Aug 2022 16:55:25 +0200 Subject: [PATCH 2/9] Added defaultEqualityCheck to createFacet Added createStaticFacet --- packages/@react-facet/core/src/createFacetContext.tsx | 4 ++-- packages/@react-facet/core/src/facet/createFacet.ts | 6 +++++- .../src/facet/{createFakeFacet.ts => createStaticFacet.ts} | 2 +- packages/@react-facet/core/src/facet/index.ts | 1 + 4 files changed, 9 insertions(+), 4 deletions(-) rename packages/@react-facet/core/src/facet/{createFakeFacet.ts => createStaticFacet.ts} (89%) diff --git a/packages/@react-facet/core/src/createFacetContext.tsx b/packages/@react-facet/core/src/createFacetContext.tsx index 8580fa35..aff7adbd 100644 --- a/packages/@react-facet/core/src/createFacetContext.tsx +++ b/packages/@react-facet/core/src/createFacetContext.tsx @@ -1,8 +1,8 @@ -import { createFakeFacet } from './facet/createFakeFacet' +import { createStaticFacet } from './facet/createStaticFacet' import { createContext } from 'react' export function createFacetContext(initialValue: T) { - const facet = createFakeFacet(initialValue) + const facet = createStaticFacet(initialValue) const context = createContext(facet) return context } diff --git a/packages/@react-facet/core/src/facet/createFacet.ts b/packages/@react-facet/core/src/facet/createFacet.ts index e8535e68..60f9b1e4 100644 --- a/packages/@react-facet/core/src/facet/createFacet.ts +++ b/packages/@react-facet/core/src/facet/createFacet.ts @@ -13,7 +13,11 @@ export interface FacetOptions { equalityCheck?: EqualityCheck } -export function createFacet({ initialValue, startSubscription, equalityCheck }: FacetOptions): WritableFacet { +export function createFacet({ + initialValue, + startSubscription, + equalityCheck = defaultEqualityCheck, +}: FacetOptions): WritableFacet { const listeners: Set> = new Set() let currentValue = initialValue let cleanupSubscription: Cleanup | undefined diff --git a/packages/@react-facet/core/src/facet/createFakeFacet.ts b/packages/@react-facet/core/src/facet/createStaticFacet.ts similarity index 89% rename from packages/@react-facet/core/src/facet/createFakeFacet.ts rename to packages/@react-facet/core/src/facet/createStaticFacet.ts index 0a121901..25d56aee 100644 --- a/packages/@react-facet/core/src/facet/createFakeFacet.ts +++ b/packages/@react-facet/core/src/facet/createStaticFacet.ts @@ -2,7 +2,7 @@ import { Facet } from '..' /** * Creates a nonwritable barebones facet to be used when you need an initial facet value that's meant to be replaced later by a real facet. Ex: with `useContext()` */ -export function createFakeFacet(value: T): Facet { +export function createStaticFacet(value: T): Facet { const facet: Facet = { get: () => value, observe: (listener) => { diff --git a/packages/@react-facet/core/src/facet/index.ts b/packages/@react-facet/core/src/facet/index.ts index 5259c030..4da7cc30 100644 --- a/packages/@react-facet/core/src/facet/index.ts +++ b/packages/@react-facet/core/src/facet/index.ts @@ -1,2 +1,3 @@ export * from './createFacet' export * from './createReadOnlyFacet' +export * from './createStaticFacet' From a873dfb2e407929e30faf61b17d7404106ed2086 Mon Sep 17 00:00:00 2001 From: Omar ElGaml Date: Thu, 25 Aug 2022 17:00:43 +0200 Subject: [PATCH 3/9] Added description to createFacet and createStaticFacet --- packages/@react-facet/core/src/facet/createFacet.ts | 4 +++- packages/@react-facet/core/src/facet/createStaticFacet.ts | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/@react-facet/core/src/facet/createFacet.ts b/packages/@react-facet/core/src/facet/createFacet.ts index 60f9b1e4..c3355afb 100644 --- a/packages/@react-facet/core/src/facet/createFacet.ts +++ b/packages/@react-facet/core/src/facet/createFacet.ts @@ -12,7 +12,9 @@ export interface FacetOptions { startSubscription?: StartSubscription equalityCheck?: EqualityCheck } - +/** + * The low level function to create a Facet, not recommended to be used if you can use any of the react facet hooks to create facets instead (Ex: useFacetState, useFacetWrap) + */ export function createFacet({ initialValue, startSubscription, diff --git a/packages/@react-facet/core/src/facet/createStaticFacet.ts b/packages/@react-facet/core/src/facet/createStaticFacet.ts index 25d56aee..55515dac 100644 --- a/packages/@react-facet/core/src/facet/createStaticFacet.ts +++ b/packages/@react-facet/core/src/facet/createStaticFacet.ts @@ -1,6 +1,7 @@ import { Facet } from '..' /** - * Creates a nonwritable barebones facet to be used when you need an initial facet value that's meant to be replaced later by a real facet. Ex: with `useContext()` + * Creates a nonwritable barebones static facet to be used when you need an initial facet value outside the react context + * that's meant to be replaced later by a real facet. Ex: with `createContext()` */ export function createStaticFacet(value: T): Facet { const facet: Facet = { From d6a8e5ce9dee53da4bdeb591aec6d0b40aebc78a Mon Sep 17 00:00:00 2001 From: Omar ElGaml Date: Thu, 25 Aug 2022 17:04:48 +0200 Subject: [PATCH 4/9] Updated console log --- packages/@react-facet/core/src/facet/createStaticFacet.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-facet/core/src/facet/createStaticFacet.ts b/packages/@react-facet/core/src/facet/createStaticFacet.ts index 55515dac..98f8f490 100644 --- a/packages/@react-facet/core/src/facet/createStaticFacet.ts +++ b/packages/@react-facet/core/src/facet/createStaticFacet.ts @@ -8,7 +8,7 @@ export function createStaticFacet(value: T): Facet { get: () => value, observe: (listener) => { if (process.env.NODE_ENV !== 'production') { - console.log(`Accessing a fake facet, perhaps you're missing a Context Provider?`) + console.log(`Accessing a static facet, perhaps you're missing a Context Provider?`) } listener(value) return () => {} From 7694287dac6851f7b5721b75a79a09c18058b31e Mon Sep 17 00:00:00 2001 From: Omar ElGaml Date: Mon, 29 Aug 2022 11:42:17 +0200 Subject: [PATCH 5/9] Added unit tests for createStaticFacet --- .../core/src/facet/createStaticFacet.spec.ts | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 packages/@react-facet/core/src/facet/createStaticFacet.spec.ts diff --git a/packages/@react-facet/core/src/facet/createStaticFacet.spec.ts b/packages/@react-facet/core/src/facet/createStaticFacet.spec.ts new file mode 100644 index 00000000..388d5f46 --- /dev/null +++ b/packages/@react-facet/core/src/facet/createStaticFacet.spec.ts @@ -0,0 +1,46 @@ +import { createStaticFacet } from './createStaticFacet' + +describe('createStaticFacet', () => { + const env = process.env + + beforeEach(() => { + jest.resetModules() + process.env = { ...env } + }) + + afterEach(() => { + process.env = env + }) + + it(`it can be read but not mutated`, () => { + const initialValue = {} + const mock = createStaticFacet(initialValue) + + expect(mock.get()).toBe(initialValue) + expect('set' in mock).toBe(false) + }) + + it(`it responds with the same value if you observe it and warns you in a non-production environment`, () => { + const consoleLogMock = jest.spyOn(console, 'log').mockImplementation() + + const update = jest.fn() + const initialValue = {} + const mock = createStaticFacet(initialValue) + + mock.observe(update) + expect(update).toHaveBeenCalledTimes(1) + expect(update).toHaveBeenCalledWith(initialValue) + expect(consoleLogMock).toHaveBeenCalledTimes(1) + expect(consoleLogMock).toHaveBeenCalledWith(`Accessing a static facet, perhaps you're missing a Context Provider?`) + + update.mockClear() + consoleLogMock.mockClear() + + process.env.NODE_ENV = 'production' + + mock.observe(update) + expect(update).toHaveBeenCalledTimes(1) + expect(update).toHaveBeenCalledWith(initialValue) + expect(consoleLogMock).toHaveBeenCalledTimes(0) + }) +}) From 58aca9844c1d9f682c522abccbd6f3bb0dffb7fa Mon Sep 17 00:00:00 2001 From: Omar ElGaml Date: Mon, 29 Aug 2022 11:47:12 +0200 Subject: [PATCH 6/9] Update createFacet tests to assume defaultEqualityCheck is passed by default --- .../core/src/facet/createFacet.spec.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/@react-facet/core/src/facet/createFacet.spec.ts b/packages/@react-facet/core/src/facet/createFacet.spec.ts index 5c0fa202..57ef809b 100644 --- a/packages/@react-facet/core/src/facet/createFacet.spec.ts +++ b/packages/@react-facet/core/src/facet/createFacet.spec.ts @@ -1,5 +1,4 @@ import 'react' -import { defaultEqualityCheck } from '../equalityChecks' import { NO_VALUE } from '../types' import { createFacet } from './createFacet' @@ -8,7 +7,7 @@ describe('equalityChecks', () => { it('fires for object values, since it can be mutated', () => { const update = jest.fn() const initialValue = {} - const mock = createFacet({ initialValue, equalityCheck: defaultEqualityCheck }) + const mock = createFacet({ initialValue }) mock.observe(update) expect(update).toHaveBeenCalledTimes(1) expect(update).toHaveBeenCalledWith(initialValue) @@ -22,7 +21,7 @@ describe('equalityChecks', () => { it('fires for array values, since it can be mutated', () => { const update = jest.fn() const initialValue: string[] = [] - const mock = createFacet({ initialValue, equalityCheck: defaultEqualityCheck }) + const mock = createFacet({ initialValue }) mock.observe(update) expect(update).toHaveBeenCalledTimes(1) expect(update).toHaveBeenCalledWith(initialValue) @@ -36,7 +35,7 @@ describe('equalityChecks', () => { it('does not fire for string', () => { const update = jest.fn() const initialValue = 'string' - const mock = createFacet({ initialValue, equalityCheck: defaultEqualityCheck }) + const mock = createFacet({ initialValue }) mock.observe(update) expect(update).toHaveBeenCalledTimes(1) expect(update).toHaveBeenCalledWith(initialValue) @@ -49,7 +48,7 @@ describe('equalityChecks', () => { it('does not fire for boolean', () => { const update = jest.fn() const initialValue = true - const mock = createFacet({ initialValue, equalityCheck: defaultEqualityCheck }) + const mock = createFacet({ initialValue }) mock.observe(update) expect(update).toHaveBeenCalledTimes(1) expect(update).toHaveBeenCalledWith(initialValue) @@ -62,7 +61,7 @@ describe('equalityChecks', () => { it('does not fire for number', () => { const update = jest.fn() const initialValue = 1 - const mock = createFacet({ initialValue, equalityCheck: defaultEqualityCheck }) + const mock = createFacet({ initialValue }) mock.observe(update) expect(update).toHaveBeenCalledTimes(1) expect(update).toHaveBeenCalledWith(initialValue) @@ -75,7 +74,7 @@ describe('equalityChecks', () => { it('does not fire for null', () => { const update = jest.fn() const initialValue = null - const mock = createFacet({ initialValue, equalityCheck: defaultEqualityCheck }) + const mock = createFacet({ initialValue }) mock.observe(update) expect(update).toHaveBeenCalledTimes(1) expect(update).toHaveBeenCalledWith(initialValue) @@ -88,7 +87,7 @@ describe('equalityChecks', () => { it('does not fire for undefined', () => { const update = jest.fn() const initialValue = undefined - const mock = createFacet({ initialValue, equalityCheck: defaultEqualityCheck }) + const mock = createFacet({ initialValue }) mock.observe(update) expect(update).toHaveBeenCalledTimes(1) expect(update).toHaveBeenCalledWith(initialValue) @@ -101,7 +100,7 @@ describe('equalityChecks', () => { it('fires if the primitive value changed', () => { const update = jest.fn() const initialValue = 'initial' - const mock = createFacet({ initialValue, equalityCheck: defaultEqualityCheck }) + const mock = createFacet({ initialValue }) mock.observe(update) expect(update).toHaveBeenCalledTimes(1) expect(update).toHaveBeenCalledWith(initialValue) From 8b0c94f8449e03cdbe039aab1226733b31c14bf1 Mon Sep 17 00:00:00 2001 From: Omar ElGaml Date: Wed, 31 Aug 2022 09:39:22 +0200 Subject: [PATCH 7/9] removed warning from createStaticFacet and kept it in createFacetContext --- .../@react-facet/core/src/createFacetContext.tsx | 15 +++++++++++++-- .../core/src/facet/createStaticFacet.ts | 3 --- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/@react-facet/core/src/createFacetContext.tsx b/packages/@react-facet/core/src/createFacetContext.tsx index aff7adbd..6fa439c6 100644 --- a/packages/@react-facet/core/src/createFacetContext.tsx +++ b/packages/@react-facet/core/src/createFacetContext.tsx @@ -1,8 +1,19 @@ -import { createStaticFacet } from './facet/createStaticFacet' import { createContext } from 'react' +import { Facet } from './types' export function createFacetContext(initialValue: T) { - const facet = createStaticFacet(initialValue) + const facet: Facet = { + get: () => initialValue, + observe: (listener) => { + if (process.env.NODE_ENV !== 'production') { + console.log( + `Accessing a static facet created through createFacetContext, perhaps you're missing a Context Provider?`, + ) + } + listener(initialValue) + return () => {} + }, + } const context = createContext(facet) return context } diff --git a/packages/@react-facet/core/src/facet/createStaticFacet.ts b/packages/@react-facet/core/src/facet/createStaticFacet.ts index 98f8f490..88001ade 100644 --- a/packages/@react-facet/core/src/facet/createStaticFacet.ts +++ b/packages/@react-facet/core/src/facet/createStaticFacet.ts @@ -7,9 +7,6 @@ export function createStaticFacet(value: T): Facet { const facet: Facet = { get: () => value, observe: (listener) => { - if (process.env.NODE_ENV !== 'production') { - console.log(`Accessing a static facet, perhaps you're missing a Context Provider?`) - } listener(value) return () => {} }, From be09b829fbcd8a6f15d5052f2db56a285ac555a6 Mon Sep 17 00:00:00 2001 From: Omar ElGaml Date: Wed, 31 Aug 2022 10:36:18 +0200 Subject: [PATCH 8/9] Updated tests for createStaticFacet and createFacetContext --- .../core/src/createFacetContext.spec.tsx | 58 ++++++++++++++++++- .../core/src/facet/createStaticFacet.spec.ts | 19 ------ 2 files changed, 57 insertions(+), 20 deletions(-) diff --git a/packages/@react-facet/core/src/createFacetContext.spec.tsx b/packages/@react-facet/core/src/createFacetContext.spec.tsx index 08fc2eb1..53fb9b94 100644 --- a/packages/@react-facet/core/src/createFacetContext.spec.tsx +++ b/packages/@react-facet/core/src/createFacetContext.spec.tsx @@ -2,7 +2,7 @@ import React, { useContext } from 'react' import { act, render } from '@react-facet/dom-fiber-testing-library' import { createFacetContext } from './createFacetContext' import { useFacetState } from './hooks' -import { Setter } from './types' +import { Facet, Setter } from './types' it(`has default value`, () => { const defaultValue = 'defaultValue' @@ -71,3 +71,59 @@ it(`it updates the context value without re-conciliation`, () => { expect(result.baseElement).toContainHTML(evenNewerValue) expect(hasRenderedMock).toBeCalledTimes(1) }) + +// Tests that the intial value of the createFacetContext is static +describe('createFacetContext initial facet', () => { + const env = process.env + let staticFacet: Facet + const defaultValue = 'defaultValue' + + beforeEach(() => { + jest.resetModules() + process.env = { ...env } + + const context = createFacetContext(defaultValue) + const Component = () => { + const stringFacet = useContext(context) + staticFacet = stringFacet + return + } + + render() + }) + + afterEach(() => { + process.env = env + }) + + it(`it can be read but not mutated`, () => { + act(() => { + expect(staticFacet.get()).toBe(defaultValue) + expect('set' in staticFacet).toBe(false) + }) + }) + + it(`it responds with the same value if you observe it and warns you in a non-production environment`, () => { + const consoleLogMock = jest.spyOn(console, 'log').mockImplementation() + + const update = jest.fn() + + staticFacet.observe(update) + expect(update).toHaveBeenCalledTimes(1) + expect(update).toHaveBeenCalledWith(defaultValue) + expect(consoleLogMock).toHaveBeenCalledTimes(1) + expect(consoleLogMock).toHaveBeenCalledWith( + `Accessing a static facet created through createFacetContext, perhaps you're missing a Context Provider?`, + ) + + update.mockClear() + consoleLogMock.mockClear() + + process.env.NODE_ENV = 'production' + + staticFacet.observe(update) + expect(update).toHaveBeenCalledTimes(1) + expect(update).toHaveBeenCalledWith(defaultValue) + expect(consoleLogMock).toHaveBeenCalledTimes(0) + }) +}) diff --git a/packages/@react-facet/core/src/facet/createStaticFacet.spec.ts b/packages/@react-facet/core/src/facet/createStaticFacet.spec.ts index 388d5f46..a5876401 100644 --- a/packages/@react-facet/core/src/facet/createStaticFacet.spec.ts +++ b/packages/@react-facet/core/src/facet/createStaticFacet.spec.ts @@ -1,17 +1,6 @@ import { createStaticFacet } from './createStaticFacet' describe('createStaticFacet', () => { - const env = process.env - - beforeEach(() => { - jest.resetModules() - process.env = { ...env } - }) - - afterEach(() => { - process.env = env - }) - it(`it can be read but not mutated`, () => { const initialValue = {} const mock = createStaticFacet(initialValue) @@ -21,8 +10,6 @@ describe('createStaticFacet', () => { }) it(`it responds with the same value if you observe it and warns you in a non-production environment`, () => { - const consoleLogMock = jest.spyOn(console, 'log').mockImplementation() - const update = jest.fn() const initialValue = {} const mock = createStaticFacet(initialValue) @@ -30,17 +17,11 @@ describe('createStaticFacet', () => { mock.observe(update) expect(update).toHaveBeenCalledTimes(1) expect(update).toHaveBeenCalledWith(initialValue) - expect(consoleLogMock).toHaveBeenCalledTimes(1) - expect(consoleLogMock).toHaveBeenCalledWith(`Accessing a static facet, perhaps you're missing a Context Provider?`) update.mockClear() - consoleLogMock.mockClear() - - process.env.NODE_ENV = 'production' mock.observe(update) expect(update).toHaveBeenCalledTimes(1) expect(update).toHaveBeenCalledWith(initialValue) - expect(consoleLogMock).toHaveBeenCalledTimes(0) }) }) From bdb8f24fa0b9b6be7d3631168ebe1bbed090a82e Mon Sep 17 00:00:00 2001 From: Omar ElGaml Date: Wed, 31 Aug 2022 11:00:39 +0200 Subject: [PATCH 9/9] Added methods to the exposed APIs test --- packages/@react-facet/core/src/index.spec.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/@react-facet/core/src/index.spec.ts b/packages/@react-facet/core/src/index.spec.ts index 94cd855c..7fd51ef1 100644 --- a/packages/@react-facet/core/src/index.spec.ts +++ b/packages/@react-facet/core/src/index.spec.ts @@ -10,9 +10,14 @@ describe('regression testing preventing accidental removal of APIs', () => { it('exposes the core facets', () => { expect(facet.createFacet).toBeDefined() + expect(facet.createStaticFacet).toBeDefined() expect(facet.createReadOnlyFacet).toBeDefined() }) + it('exposes the react facet methods', () => { + expect(facet.createFacetContext).toBeDefined() + }) + it('exposes the hooks', () => { expect(facet.useFacetCallback).toBeDefined() expect(facet.useFacetEffect).toBeDefined()