From 937284530f34ad89b6c146927f2bba66fc1fe67c Mon Sep 17 00:00:00 2001 From: Damien Clarke Date: Mon, 28 Feb 2022 10:05:59 +1100 Subject: [PATCH 1/3] feat: forms on impossible / missing values are now readonly --- packages/dendriform/src/Dendriform.tsx | 6 ++++-- packages/dendriform/test/Dendriform.test.tsx | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/dendriform/src/Dendriform.tsx b/packages/dendriform/src/Dendriform.tsx index a40c195..8d835f9 100644 --- a/packages/dendriform/src/Dendriform.tsx +++ b/packages/dendriform/src/Dendriform.tsx @@ -316,8 +316,10 @@ export class Core { }); } - const id = node ? node.id : 'notfound'; - return this.getFormById(id, readonly); + if(!node) { + return this.getFormById('notfound', true); + } + return this.getFormById(node.id, readonly); }; getFormById = (id: string, readonly: boolean): Dendriform => { diff --git a/packages/dendriform/test/Dendriform.test.tsx b/packages/dendriform/test/Dendriform.test.tsx index 619e63f..9f60812 100644 --- a/packages/dendriform/test/Dendriform.test.tsx +++ b/packages/dendriform/test/Dendriform.test.tsx @@ -791,6 +791,7 @@ describe(`Dendriform`, () => { expect(barForm.value).toBe(undefined); expect(barForm.id).toBe('notfound'); + expect(barForm._readonly).toBe(true); }); test(`should get deleted child value`, () => { @@ -803,6 +804,7 @@ describe(`Dendriform`, () => { form.set([[[123]]]); expect(elemForm.branch(0).value).toBe(undefined); + expect(elemForm.branch(0)._readonly).toBe(true); expect(form.branch([0,0,0]).value).toBe(123); }); }); From ca46ce1b835f967752c20fe5eac025b38f6656b7 Mon Sep 17 00:00:00 2001 From: Damien Clarke Date: Mon, 28 Feb 2022 10:25:58 +1100 Subject: [PATCH 2/3] feat: make api consistent by not throwing errors on renderAll / branchAll --- packages/dendriform/src/Dendriform.tsx | 9 ++++----- packages/dendriform/src/errors.ts | 6 +++--- packages/dendriform/test/Dendriform.test.tsx | 15 ++++++--------- packages/dendriform/test/errors.test.ts | 6 +++--- 4 files changed, 16 insertions(+), 20 deletions(-) diff --git a/packages/dendriform/src/Dendriform.tsx b/packages/dendriform/src/Dendriform.tsx index 8d835f9..9184686 100644 --- a/packages/dendriform/src/Dendriform.tsx +++ b/packages/dendriform/src/Dendriform.tsx @@ -26,7 +26,6 @@ import produce, {isDraft, original} from 'immer'; import {producePatches, Patch} from './producePatches'; import type {ToProduce} from './producePatches'; import {die} from './errors'; -import type {ErrorKey} from './errors'; import {newNode, addNode, getPath, getNodeByPath, produceNodePatches, getNode} from './Nodes'; import type {Nodes, NodeAny, NewNodeCreator} from './Nodes'; import type {Plugin} from './Plugin'; @@ -716,11 +715,11 @@ const Branch = React.memo( const branchable = (thing: any) => getType(thing) !== BASIC; // eslint-disable-next-line @typescript-eslint/no-explicit-any -const entriesOrDie = (thing: any, error: ErrorKey) => { +const entriesOrNone = (thing: any): any[] => { try { return entries(thing); } catch(e) { - die(error); + return []; } }; @@ -967,7 +966,7 @@ export class Dendriform { branchAll(pathOrKey: any): any { const got = this.branch(pathOrKey); // eslint-disable-next-line @typescript-eslint/no-explicit-any - return entriesOrDie(got.value, 2).map(([key]) => got.branch(key as any)); + return entriesOrNone(got.value).map(([key]) => got.branch(key as any)); } render, K2 extends keyof Val, K3 extends keyof Val,K2>, K4 extends keyof Val,K2>,K3>>(path: [K1, K2, K3, K4], renderer: Renderer,K2>,K3>[K4],P>>, deps?: unknown[]): React.ReactElement; @@ -1002,7 +1001,7 @@ export class Dendriform { const containerRenderer = (): React.ReactElement[] => { const value = form.useValue(); - return entriesOrDie(value, 3).map(([key]): React.ReactElement => { + return entriesOrNone(value).map(([key]): React.ReactElement => { // eslint-disable-next-line @typescript-eslint/no-explicit-any const child = form.branch(key as any); return renderer(child)} deps={deps} />; diff --git a/packages/dendriform/src/errors.ts b/packages/dendriform/src/errors.ts index eae1b16..e74fde9 100644 --- a/packages/dendriform/src/errors.ts +++ b/packages/dendriform/src/errors.ts @@ -1,10 +1,10 @@ -const all = `can only be called on forms containing an array, object, es6 map or es6 set`; +// const all = `can only be called on forms containing an array, object, es6 map or es6 set`; const errors = { 0: (id: number) => `Cannot find path of node ${id}`, // 1: (path: unknown[]) => `Cannot find node at path ${path.map(a => JSON.stringify(a)).join('","')}`, - 2: `branchAll() ${all}`, - 3: `renderAll() ${all}`, + // 2: `branchAll() ${all}`, + // 3: `renderAll() ${all}`, 4: (path: unknown[]) => `useIndex() can only be called on array element forms, can't be called at path ${path.map(a => JSON.stringify(a)).join('","')}`, 5: `sync() forms must have the same maximum number of history items configured`, 6: (msg: string) => `onDerive() callback must not throw errors on first call. Threw: ${msg}`, diff --git a/packages/dendriform/test/Dendriform.test.tsx b/packages/dendriform/test/Dendriform.test.tsx index 9f60812..1bd83cf 100644 --- a/packages/dendriform/test/Dendriform.test.tsx +++ b/packages/dendriform/test/Dendriform.test.tsx @@ -887,10 +887,10 @@ describe(`Dendriform`, () => { expect(forms.map(f => f.value)).toEqual([0,1]); }); - test(`should error if getting a basic type`, () => { + test(`should NOT error if getting a basic type`, () => { const form = new Dendriform(123); - expect(() => form.branchAll()).toThrow('branchAll() can only be called on forms containing an array, object, es6 map or es6 set'); + expect(form.branchAll()).toEqual([]); }); // TODO what about misses? @@ -1176,11 +1176,7 @@ describe(`Dendriform`, () => { describe(`rendering`, () => { - test(`should error if rendering a basic type`, () => { - const consoleError = console.error; - // eslint-disable-next-line @typescript-eslint/no-empty-function - console.error = () => {}; - + test(`should NOT error if rendering a basic type`, () => { const form = new Dendriform('4'); const renderer = jest.fn(form =>
{form.value}
); @@ -1189,9 +1185,10 @@ describe(`Dendriform`, () => { return props.form.renderAll(renderer); }; - expect(() => mount()).toThrow('renderAll() can only be called on forms containing an array, object, es6 map or es6 set'); + const wrapper = mount(); - console.error = consoleError; + expect(renderer).toHaveBeenCalledTimes(0); + expect(wrapper.find('.branch').length).toBe(0); }); test(`should renderAll no levels and return React element`, () => { diff --git a/packages/dendriform/test/errors.test.ts b/packages/dendriform/test/errors.test.ts index c5973f5..989b50f 100644 --- a/packages/dendriform/test/errors.test.ts +++ b/packages/dendriform/test/errors.test.ts @@ -4,8 +4,8 @@ describe(`die`, () => { test(`should throw errors`, () => { expect(() => die(0, 123)).toThrow(`[Dendriform] Cannot find path of node 123`); // expect(() => die(1, ['a',1])).toThrow(`[Dendriform] Cannot find node at path ["a",1]`); - expect(() => die(2)).toThrow(`[Dendriform] branchAll() can only be called on forms containing an array, object, es6 map or es6 set`); - expect(() => die(3)).toThrow(`[Dendriform] renderAll() can only be called on forms containing an array, object, es6 map or es6 set`); + // expect(() => die(2)).toThrow(`[Dendriform] branchAll() can only be called on forms containing an array, object, es6 map or es6 set`); + // expect(() => die(3)).toThrow(`[Dendriform] renderAll() can only be called on forms containing an array, object, es6 map or es6 set`); expect(() => die(4, ['foo'])).toThrow(`[Dendriform] useIndex() can only be called on array element forms, can't be called at path [\"foo\"]`); }); @@ -19,7 +19,7 @@ describe(`die (prod mode)`, () => { test(`should throw minified errors`, () => { global.__DEV__ = false; expect(() => die(0, 123, "woo")).toThrow(`[Dendriform] minified error #0: 123, "woo"`); - expect(() => die(2)).toThrow(`[Dendriform] minified error #2:`); + expect(() => die(4)).toThrow(`[Dendriform] minified error #4:`); }); }); From 8e7c4e309c855ae587a25466758e270f2dabd1c4 Mon Sep 17 00:00:00 2001 From: Damien Clarke Date: Mon, 28 Feb 2022 10:44:23 +1100 Subject: [PATCH 3/3] docs: update docs to cover expected edge case behaviour --- README.md | 25 ++++++++++++++++++++++--- packages/dendriform/README.md | 25 ++++++++++++++++++++++--- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index dda76ed..bacd286 100644 --- a/README.md +++ b/README.md @@ -138,8 +138,9 @@ npm install --save dendriform - [Creation](#creation) - [Values](#values) - [Branching](#branching) +- [Branching multiple children](#branching-multiple-children) - [Rendering](#rendering) -- [Rendering arrays](#rendering-arrays) +- [Rendering arrays and multiple children](#rendering-arrays-and-multiple-children) - [Setting data](#setting-data) - [Read-only forms](#readonly-forms) - [Updating from props](#updating-from-props) @@ -274,15 +275,29 @@ function MyComponent(props) { [Demo](http://dendriform.xyz#branch) -A form containing a non-branchable value such as a string, number, undefined or null will throw an error if `.branch()` is called on it. You can check if a form is branchable using `.branchable`: +You can check if a form is branchable using `.branchable`. On a form containing a non-branchable value such as a string, number, undefined or null it will return false, or if the form is branchable it will return true. ```js new Dendriform(123).branchable; // returns false new Dendriform({name: 'Bill'}).branchable; // returns true ``` +You can still call `.branch()` on non-branchable forms - the returned form will be read-only and contain a value of undefined. While this may seem overly loose, it is to prevent the proliferation of safe-guarding code in userland, and is useful for situations where React components that render branched forms are still briefly mounted after a parent values changes from a branchable type to a non-branchable type. + +### Branching multiple children + The `.branchAll()` methods can be used to branch all children at once, returning an array of branched forms. +```js +const form = new Dendriform(['a','b','c']); + +const elementForms = form.branchAll(); +// elementForms.length is 3 +// elementForms[0].value is 'a' +``` + +You can still call `.branchAll()` on non-branchable or non-iterable forms - it will return an empty array in this case. + ### Rendering The `.render()` function allows you to branch off and render a deep value in a React component. @@ -404,7 +419,9 @@ function MyComponent(props) { [Demo](http://dendriform.xyz#renderdeps) -### Rendering arrays +You can still call `.render()` on non-branchable forms - the returned form will be read-only and contain a value of undefined. While this may seem overly loose, it is to prevent the proliferation of safe-guarding code in userland, and is useful for situations where React components that render branched forms are still briefly mounted after a parent values changes from a branchable type to a non-branchable type. + +### Rendering arrays and multiple children The `.renderAll()` function works in the same way as `.render()`, but repeats for all elements in an array. React keying is taken care of for you. @@ -463,6 +480,8 @@ const petName = form.branch(['pets', 0, 'name']); Like with `.render()`, the `.renderAll()` function can also additionally accept an array of dependencies that will cause it to update in response to prop changes. +You can still call `.renderAll()` on non-branchable or non-iterable forms - it will return an empty array in this case. + ### Setting data You can set data directly using `.set()`. This accepts the new value for the form. When called, changes will immediately be applied to the data in the form and any relevant `.useValue()` hooks and `.render()` methods will be scheduled to update by React. diff --git a/packages/dendriform/README.md b/packages/dendriform/README.md index dda76ed..bacd286 100644 --- a/packages/dendriform/README.md +++ b/packages/dendriform/README.md @@ -138,8 +138,9 @@ npm install --save dendriform - [Creation](#creation) - [Values](#values) - [Branching](#branching) +- [Branching multiple children](#branching-multiple-children) - [Rendering](#rendering) -- [Rendering arrays](#rendering-arrays) +- [Rendering arrays and multiple children](#rendering-arrays-and-multiple-children) - [Setting data](#setting-data) - [Read-only forms](#readonly-forms) - [Updating from props](#updating-from-props) @@ -274,15 +275,29 @@ function MyComponent(props) { [Demo](http://dendriform.xyz#branch) -A form containing a non-branchable value such as a string, number, undefined or null will throw an error if `.branch()` is called on it. You can check if a form is branchable using `.branchable`: +You can check if a form is branchable using `.branchable`. On a form containing a non-branchable value such as a string, number, undefined or null it will return false, or if the form is branchable it will return true. ```js new Dendriform(123).branchable; // returns false new Dendriform({name: 'Bill'}).branchable; // returns true ``` +You can still call `.branch()` on non-branchable forms - the returned form will be read-only and contain a value of undefined. While this may seem overly loose, it is to prevent the proliferation of safe-guarding code in userland, and is useful for situations where React components that render branched forms are still briefly mounted after a parent values changes from a branchable type to a non-branchable type. + +### Branching multiple children + The `.branchAll()` methods can be used to branch all children at once, returning an array of branched forms. +```js +const form = new Dendriform(['a','b','c']); + +const elementForms = form.branchAll(); +// elementForms.length is 3 +// elementForms[0].value is 'a' +``` + +You can still call `.branchAll()` on non-branchable or non-iterable forms - it will return an empty array in this case. + ### Rendering The `.render()` function allows you to branch off and render a deep value in a React component. @@ -404,7 +419,9 @@ function MyComponent(props) { [Demo](http://dendriform.xyz#renderdeps) -### Rendering arrays +You can still call `.render()` on non-branchable forms - the returned form will be read-only and contain a value of undefined. While this may seem overly loose, it is to prevent the proliferation of safe-guarding code in userland, and is useful for situations where React components that render branched forms are still briefly mounted after a parent values changes from a branchable type to a non-branchable type. + +### Rendering arrays and multiple children The `.renderAll()` function works in the same way as `.render()`, but repeats for all elements in an array. React keying is taken care of for you. @@ -463,6 +480,8 @@ const petName = form.branch(['pets', 0, 'name']); Like with `.render()`, the `.renderAll()` function can also additionally accept an array of dependencies that will cause it to update in response to prop changes. +You can still call `.renderAll()` on non-branchable or non-iterable forms - it will return an empty array in this case. + ### Setting data You can set data directly using `.set()`. This accepts the new value for the form. When called, changes will immediately be applied to the data in the form and any relevant `.useValue()` hooks and `.render()` methods will be scheduled to update by React.