-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
fix: make immutable option work more correctly #13526
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'svelte': patch | ||
--- | ||
|
||
fix: make immutable option work more correctly |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,12 +67,15 @@ export function state(v) { | |
/** | ||
* @template V | ||
* @param {V} initial_value | ||
* @param {boolean} [immutable] | ||
* @returns {Source<V>} | ||
*/ | ||
/*#__NO_SIDE_EFFECTS__*/ | ||
export function mutable_source(initial_value) { | ||
export function mutable_source(initial_value, immutable = false) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than doing this, can't we just use the non-mutable source instead? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, because this method additionally adds the source to the before/afterUpdate array, which is necessary in legacy mode |
||
const s = source(initial_value); | ||
s.equals = safe_equals; | ||
if (!immutable) { | ||
s.equals = safe_equals; | ||
} | ||
|
||
// bind the signal to the component context, in case we need to | ||
// track updates to trigger beforeUpdate/afterUpdate callbacks | ||
|
@@ -86,10 +89,11 @@ export function mutable_source(initial_value) { | |
/** | ||
* @template V | ||
* @param {V} v | ||
* @param {boolean} [immutable] | ||
* @returns {Source<V>} | ||
*/ | ||
export function mutable_state(v) { | ||
return push_derived_source(mutable_source(v)); | ||
export function mutable_state(v, immutable = false) { | ||
return push_derived_source(mutable_source(v, immutable)); | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
<svelte:options immutable /> | ||
|
||
<script> | ||
import { afterUpdate, beforeUpdate } from 'svelte'; | ||
|
||
export let todo; | ||
|
||
let btn; | ||
|
||
$: console.log('$:'+ todo.id); | ||
|
||
beforeUpdate(() => { | ||
console.log('beforeUpdate:'+ todo.id); | ||
}) | ||
|
||
afterUpdate(() => { | ||
console.log('afterUpdate:'+ todo.id); | ||
}); | ||
</script> | ||
|
||
<button bind:this={btn} on:click> | ||
{todo.done ? 'X' : ''} | ||
{todo.id} | ||
</button> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
import { flushSync } from 'svelte'; | ||
import { test } from '../../test'; | ||
|
||
export default test({ | ||
immutable: true, | ||
|
||
html: '<button>1</button> <button>2</button> <button>3</button>', | ||
|
||
test({ assert, target, logs }) { | ||
assert.deepEqual(logs, [ | ||
'$:1', | ||
'beforeUpdate:1', | ||
'$:2', | ||
'beforeUpdate:2', | ||
'$:3', | ||
'beforeUpdate:3', | ||
'afterUpdate:1', | ||
'afterUpdate:2', | ||
'afterUpdate:3', | ||
'beforeUpdate:1', | ||
'beforeUpdate:2', | ||
'beforeUpdate:3' | ||
]); | ||
|
||
const [button1, button2] = target.querySelectorAll('button'); | ||
|
||
logs.length = 0; | ||
button1.click(); | ||
flushSync(); | ||
assert.htmlEqual( | ||
target.innerHTML, | ||
'<button>X 1</button> <button>2</button> <button>3</button>' | ||
); | ||
assert.deepEqual(logs, ['$:1', 'beforeUpdate:1', 'afterUpdate:1']); | ||
|
||
logs.length = 0; | ||
button2.click(); | ||
flushSync(); | ||
assert.htmlEqual( | ||
target.innerHTML, | ||
'<button>X 1</button> <button>X 2</button> <button>3</button>' | ||
); | ||
assert.deepEqual(logs, ['$:2', 'beforeUpdate:2', 'afterUpdate:2']); | ||
} | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
<script> | ||
import ImmutableTodo from './ImmutableTodo.svelte'; | ||
|
||
let todos = [ | ||
{ id: 1, done: false }, | ||
{ id: 2, done: false }, | ||
{ id: 3, done: false } | ||
]; | ||
|
||
function toggle(id) { | ||
todos = todos.map((todo) => { | ||
if (todo.id === id) { | ||
return { | ||
id, | ||
done: !todo.done | ||
}; | ||
} | ||
|
||
return todo; | ||
}); | ||
} | ||
</script> | ||
|
||
{#each todos as todo} | ||
<ImmutableTodo {todo} on:click={() => toggle(todo.id)} /> | ||
{/each} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
import { flushSync } from 'svelte'; | ||
import { test } from '../../test'; | ||
|
||
export default test({ | ||
immutable: true, | ||
|
||
html: '<button>0</button> <button>0</button>', | ||
|
||
test({ assert, target }) { | ||
const [button1, button2] = target.querySelectorAll('button'); | ||
|
||
button1.click(); | ||
flushSync(); | ||
assert.htmlEqual(target.innerHTML, '<button>0</button> <button>0</button>'); | ||
|
||
button2.click(); | ||
flushSync(); | ||
assert.htmlEqual(target.innerHTML, '<button>2</button> <button>2</button>'); | ||
} | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
<script> | ||
let name = { value: 0 }; | ||
</script> | ||
|
||
<button onclick={() => name.value++}>{name.value}</button> | ||
<button onclick={() => (name = { value: name.value + 1 })}>{name.value}</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be a source?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it's used in the derived, and the derived then does equality checks to see whether or not the version has changed. If we had the possibility of accessing the current/previous value within the derived I would do that instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oooooh gotcha...thanks for the explanation