Skip to content

Commit

Permalink
Fix: Prevent stale values when a binding's bound object changes.
Browse files Browse the repository at this point in the history
  • Loading branch information
kitschpatrol committed May 19, 2024
1 parent a6c5ad3 commit 610f3db
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 3 deletions.
67 changes: 67 additions & 0 deletions src/examples/tests/TestBindingObjectChange.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<script lang="ts">
import { Binding, List } from '$lib';
import AutoObject from '$lib/extra/AutoObject.svelte';
let testObjectA = {
someBoolean: true,
someColor: {
r: 255,
g: 0,
b: 55
},
someFolder: {
b: 2,
a: 1,
c: 3
},
someNumber: 1,
somePoint: {
x: 1,
y: 2
},
someString: 'test'
};
let testObjectB = {
someBoolean: false,
someFolder: {
r: 3,
b: 2,
a: 1,
c: 3,
d: 4,
e: 5
},
someNumber: 3,
someOtherColor: {
r: 0,
g: 55,
b: 55
},
someOtherPoint: {
x: 1,
y: 2,
z: 2,
f: 3
},
someString: 'test'
};
const testOptions = { b: testObjectA, a: testObjectB };
let activeObject = testObjectA;
$: {
console.log('activeObject', activeObject);
}
</script>

// Noticed issues with re-assigning the object when working on Tweakpane CSS with STUI version 1.2.6

<List bind:value={activeObject} options={testOptions} />
<hr />
<Binding bind:object={activeObject} key="someBoolean" />
<hr />
<Binding bind:object={activeObject} key="someNumber" />
<hr />
<AutoObject bind:object={activeObject} />
57 changes: 57 additions & 0 deletions src/examples/tests/TestOrder.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<script lang="ts">
import {
AutoValue,
type BindingObject,
ButtonGrid,
Checkbox,
Pane,
Separator,
Slider
} from '$lib';
import Button from '$lib/control/Button.svelte';
import Folder from '$lib/core/Folder.svelte';
let testObject = {
someColor: {
r: 255,
g: 0,
b: 55
},
someOtherColor: {
r: 0,
g: 255,
b: 55
}
} as BindingObject;
let showNumbers = true;
let folderWrap = false;
let someNumber = 1;
</script>

<Pane>
{#if folderWrap}
<Folder>
{#each Object.keys(testObject) as key}
{#if typeof testObject[key] !== 'number' || showNumbers}
<AutoValue bind:value={testObject[key]} label={key} />
{/if}
{/each}
</Folder>
<Slider bind:value={someNumber} label="Some Number" />
{:else}
{#each Object.keys(testObject) as key}
{#if typeof testObject[key] !== 'number' || showNumbers}
<AutoValue bind:value={testObject[key]} label={key} />
{/if}
{/each}
<Slider bind:value={someNumber} label="Some Number" />
{/if}
<Separator />
<ButtonGrid buttons={['Copy', 'Reset']} />
<Folder expanded={false} title="Tweakpane CSS Options">
<Checkbox bind:value={showNumbers} label="Show Numbers" />
<Checkbox bind:value={folderWrap} label="Folder Wrap" />
</Folder>
<Button />
</Pane>
19 changes: 16 additions & 3 deletions src/lib/core/Binding.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -159,20 +159,25 @@
const dispatch = createEventDispatcher<UnwrapCustomEvents<$$Events>>();
// Good grief...
// Good grief... can't wait for Svelte 5's fine-grained reactivity:
// Work around for double-reactivity object bug
// https://github.com/sveltejs/svelte/pull/8992
// https://github.com/sveltejs/svelte/issues/4265
// Switching to <svelte:options immutable={true} />
// at this point would be more involved
let lastObject: T = object;
let lastValue: T[keyof T] = copy(object[key]);
let internalChange = false;
function onBoundValueChange(object: T) {
// Check svelte implementation?
// TODO primitive checks for optimization?
// TODO need deep for anything?
// TODO consider completely proxy-ing Tweakpane
// object to intercept bound updates?
if (lastObject !== object) {
internalChange = false;
}
if (!shallowEqual(object[key], lastValue)) {
lastValue = copy(object[key]);
Expand All @@ -191,6 +196,12 @@
}
internalChange = false;
// Check for the bound object changing entirely...
if (lastObject !== object) {
lastObject = object;
create(); // Recreation seems to be only way to re-bind to new object
}
}
function onTweakpaneChange() {
Expand All @@ -204,10 +215,12 @@
// Options seem immutable...
// have to recreate old version supporting key changes $: key, options,
$: options, $parentStore !== undefined && index !== undefined && create();
// $: object, _ref !== undefined && $parentStore !== undefined && _ref.refresh();
$: _ref !== undefined && (_ref.disabled = disabled);
$: _ref !== undefined && (_ref.label = label);
// A refresh alone doesn't seem to be enough when the object itself (not
// just its values) has changed, so it's handled in onBoundValueChange
// $: object, _ref !== undefined && $parentStore !== undefined && _ref.refresh();
$: $parentStore !== undefined && onBoundValueChange(object);
$: theme &&
Expand Down
1 change: 1 addition & 0 deletions src/lib/extra/AutoValue.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ Plugin component behavior is not available in `<AutoValue>`.
@sourceLink
[AutoValue.svelte](https://github.com/kitschpatrol/svelte-tweakpane-ui/blob/main/src/lib/extra/AutoValue.svelte)
-->

{#if typeof value === 'string'}
<Text bind:value on:change {...$$restProps} />
{:else}
Expand Down

0 comments on commit 610f3db

Please sign in to comment.