Skip to content

Commit

Permalink
Merge pull request #85 from dxinteractive/feature/fix-primitive-optim…
Browse files Browse the repository at this point in the history 
…isation

Fix primitive array movement optimisation
  • Loading branch information
@dxinteractive
dxinteractive authored Mar 17, 2022
2 parents 33e9670 + 109e799 commit 3b721b0
Show file tree
Hide file tree
Showing 6 changed files with 289 additions and 176 deletions.
18 changes: 14 additions & 4 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -430,18 +430,24 @@ See [Array operations](#array-operations) for convenient ways to let the user ma
```js
function MyComponent(props) {
const form = useDendriform({
colours: ['Red', 'Green', 'Blue']
colours: [
{colour: 'Red'},
{colour: 'Green'},
{colour: 'Blue'}
]
});

return <div>
{form.renderAll('colours', colourForm => {
const colour = colourForm.useValue();
const colour = colourForm.branch('colour').useValue();
return <div>Colour: {colour}</div>;
})}
</div>;
}
```
Please note that if you are rendering an array that contains any primitive values (undefined, number, string) then the automatic keying will not be able to be as accurate as if you were to render an array of objects / arrays / class instances / sets / maps. This is because object references are used to track the movement of array elements through time, and primitive elements cannot be tracked in this way.
Array element forms can also opt-in to updates regarding their indexes using the `.useIndex()` hook.
If you'll be allowing users to re-order items in an array, then please note that you'll get better performance if array element components don't know about their indexes. If the `.useIndex()` hook is used, a element that has moved its position inside of its parent array will need to update, even if it is otherwise unchanged.
Expand Down Expand Up @@ -1214,7 +1220,11 @@ const dndReorder = (result) => (draft) => {
function DragAndDrop() {

const form = useDendriform({
colours: ['Red', 'Green', 'Blue']
colours: [
{colour: 'Red'},
{colour: 'Green'},
{colour: 'Blue'}
]
});

const onDragEnd = useCallback(result => {
Expand Down Expand Up @@ -1254,7 +1264,7 @@ function DragAndDropList(props) {
{...provided.draggableProps}
{...provided.dragHandleProps}
>
<label>colour: <input {...useInput(eachForm, 150)} /></label>
<label>colour: <input {...useInput(eachForm.branch('colour'), 150)} /></label>
<button onClick={remove}>remove</button>
</div>}
</Draggable>;
Expand Down
81 changes: 58 additions & 23 deletions packages/dendriform-demo/components/Demos.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -820,14 +820,18 @@ const offsetElement = <T,>(form: Dendriform<T>, offset: number): void => {
function ArrayOperations(): React.ReactElement {

const form = useDendriform({
colours: ['Red', 'Green', 'Blue']
colours: [
{colour: 'Red'},
{colour: 'Green'},
{colour: 'Blue'}
]
});

const coloursForm = form.branch('colours');
const shift = useCallback(() => coloursForm.set(array.shift()), []);
const pop = useCallback(() => coloursForm.set(array.pop()), []);
const unshift = useCallback(() => coloursForm.set(array.unshift('Puce')), []);
const push = useCallback(() => coloursForm.set(array.push('Puce')), []);
const unshift = useCallback(() => coloursForm.set(array.unshift({colour: 'Puce'})), []);
const push = useCallback(() => coloursForm.set(array.push({colour: 'Puce'})), []);
const move = useCallback(() => coloursForm.set(array.move(-1,0)), []);

return <Region>
Expand All @@ -838,8 +842,11 @@ function ArrayOperations(): React.ReactElement {
const moveUp = useCallback(() => offsetElement(colourForm, -1), []);

return <Region>
<label>colour: <input {...useInput(colourForm, 150)} /></label>

{colourForm.render('colour', form => (
<Region>
<label>colour: <input {...useInput(form, 150)} /></label>
</Region>
))}
<button type="button" onClick={remove}>remove</button>
<button type="button" onClick={moveDown}>down</button>
<button type="button" onClick={moveUp}>up</button>
Expand All @@ -861,14 +868,18 @@ const offsetElement = (form, offset) => {
function MyComponent(props) {
const form = useDendriform({
colours: ['Red', 'Green', 'Blue']
colours: [
{colour: 'Red'},
{colour: 'Green'},
{colour: 'Blue'}
]
});
const coloursForm = form.branch('colours');
const shift = useCallback(() => coloursForm.set(array.shift()), []);
const pop = useCallback(() => coloursForm.set(array.pop()), []);
const unshift = useCallback(() => coloursForm.set(array.unshift('Puce')), []);
const push = useCallback(() => coloursForm.set(array.push('Puce')), []);
const unshift = useCallback(() => coloursForm.set(array.unshift({colour: 'Puce'})), []);
const push = useCallback(() => coloursForm.set(array.push({colour: 'Puce'})), []);
const move = useCallback(() => coloursForm.set(array.move(-1,0)), []);
return <div>
Expand All @@ -879,7 +890,9 @@ function MyComponent(props) {
const moveUp = useCallback(() => offsetElement(colourForm, -1), []);
return <div>
<label>colour: <input {...useInput(colourForm, 150)} /></label>
{colourForm.render('colour', form => (
<label>colour: <input {...useInput(form, 150)} /></label>
))}
<button type="button" onClick={remove}>remove</button>
<button type="button" onClick={moveDown}>down</button>
Expand All @@ -903,12 +916,16 @@ function MyComponent(props) {
function ArrayIndexes(): React.ReactElement {

const form = useDendriform({
colours: ['Red', 'Green', 'Blue']
colours: [
{colour: 'Red'},
{colour: 'Green'},
{colour: 'Blue'}
]
});

return <Region>
{form.renderAll('colours', colourForm => {
const colour = colourForm.useValue();
const colour = colourForm.branch('colour').useValue();
const index = colourForm.useIndex();
const moveDown = useCallback(() => offsetElement(colourForm, 1), []);
const moveUp = useCallback(() => offsetElement(colourForm, -1), []);
Expand All @@ -930,12 +947,16 @@ const offsetElement = (form, offset) => {
function MyComponent(props) {
const form = useDendriform({
colours: ['Red', 'Green', 'Blue']
colours: [
{colour: 'Red'},
{colour: 'Green'},
{colour: 'Blue'}
]
});
return <div>
{form.renderAll('colours', colourForm => {
const colour = colourForm.useValue();
const colour = colourForm.branch('colour').useValue();
const index = colourForm.useIndex();
const moveDown = useCallback(() => offsetElement(colourForm, 1), []);
const moveUp = useCallback(() => offsetElement(colourForm, -1), []);
Expand Down Expand Up @@ -1402,15 +1423,19 @@ function dndReorder<V extends unknown[]>(result: DropResult): ((draft: Draft<V>)
function DragAndDrop(): React.ReactElement {

const form = useDendriform({
colours: ['Red', 'Green', 'Blue']
colours: [
{colour: 'Red'},
{colour: 'Green'},
{colour: 'Blue'}
]
});

const onDragEnd = useCallback(result => {
form.branch('colours').set(dndReorder(result));
}, []);

const onAdd = useCallback(() => {
form.branch('colours').set(array.push('Puce'));
form.branch('colours').set(array.push({colour: 'Puce'}));
}, []);

return <Region>
Expand All @@ -1431,7 +1456,7 @@ function DragAndDrop(): React.ReactElement {
}

type DragAndDropListProps = {
form: Dendriform<string[]>;
form: Dendriform<{colour: string}[]>;
};

function DragAndDropList(props: DragAndDropListProps): React.ReactElement {
Expand All @@ -1448,7 +1473,11 @@ function DragAndDropList(props: DragAndDropListProps): React.ReactElement {
{...provided.dragHandleProps}
>
<Region>
<label>colour: <input {...useInput(eachForm, 150)} /></label>
{eachForm.render('colour', form => (
<Region>
<label>colour: <input {...useInput(form, 150)} /></label>
</Region>
))}
<button type="button" onClick={remove}>remove</button>
</Region>
</div>}
Expand All @@ -1473,15 +1502,19 @@ const dndReorder = (result) => (draft) => {
function DragAndDrop() {
const form = useDendriform({
colours: ['Red', 'Green', 'Blue']
colours: [
{colour: 'Red'},
{colour: 'Green'},
{colour: 'Blue'}
]
});
const onDragEnd = useCallback(result => {
form.branch('colours').set(dndReorder(result));
}, []);
const onAdd = useCallback(() => {
form.branch('colours').set(array.push('Puce'));
form.branch('colours').set(array.push({colour: 'Puce'}));
}, []);
return <div>
Expand Down Expand Up @@ -1513,7 +1546,9 @@ function DragAndDropList(props) {
{...provided.draggableProps}
{...provided.dragHandleProps}
>
<label>colour: <input {...useInput(eachForm, 150)} /></label>
{eachForm.render('colour', form => (
<label>colour: <input {...useInput(eachForm, 150)} /></label>
))}
<button type="button" onClick={remove}>remove</button>
</div>}
</Draggable>;
Expand Down Expand Up @@ -1561,7 +1596,7 @@ function PluginSubmitExample(): React.ReactElement {
},
onError: e => e.message
})
});
});

const form = useDendriform<SubmitValue,SubmitPlugins>(() => initialValue, {plugins});

Expand Down Expand Up @@ -1632,7 +1667,7 @@ function PluginSubmitExample() {
await fakeSave(newValue);
}
})
});
});
const form = useDendriform(() => initialValue, {plugins});
Expand Down Expand Up @@ -1691,7 +1726,7 @@ function Cancel(): React.ReactElement {
}
});


// eslint-disable-next-line no-console
ageForm.useCancel(reason => console.warn(reason));

Expand Down
5 changes: 5 additions & 0 deletions packages/dendriform-immer-patch-optimiser/src/optimise.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ export const optimiseArray = <B>(base: B[], patches: ImmerPatch[]): DendriformPa
if(!Array.isArray(value)) return patches;
targetIds = value.map(addItem);

} else if(base.some(b => typeof b !== 'object')) {
// if any primitives are in the array, we cant reliably track by reference
// so skip the optimisation
return patches;

} else {
const replacedPatches = patches.map(patch => {
const {op, value, path} = patch;
Expand Down
Loading

0 comments on commit 3b721b0

Please sign in to comment.