Merge branch 'master' into pr/5839

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Svelte changelog
 
+## Unreleased
+
+* Rework SSR store handling to subscribe and unsubscribe as in DOM mode ([#3375](https://github.com/sveltejs/svelte/issues/3375), [#3582](https://github.com/sveltejs/svelte/issues/3582), [#3636](https://github.com/sveltejs/svelte/issues/3636))
+* Fix error when removing elements that are already transitioning out ([#5789](https://github.com/sveltejs/svelte/issues/5789), [#5808](https://github.com/sveltejs/svelte/issues/5808))
+* Fix duplicate content race condition with `{#await}` blocks and out transitions ([#5815](https://github.com/sveltejs/svelte/issues/5815))
+
 ## 3.31.1
 
 * Fix scrolling of element with resize listener by making the `<iframe>` have `z-index: -1` ([#5448](https://github.com/sveltejs/svelte/issues/5448))

src/compiler/compile/render_dom/Renderer.ts

@@ -6,6 +6,7 @@ import { x } from 'code-red';
 import { Node, Identifier, MemberExpression, Literal, Expression, BinaryExpression } from 'estree';
 import flatten_reference from '../utils/flatten_reference';
 import { reserved_keywords } from '../utils/reserved_keywords';
+import { renderer_invalidate } from './invalidate';
 
 interface ContextMember {
 	name: string;
@@ -168,57 +169,7 @@ export default class Renderer {
 	}
 
 	invalidate(name: string, value?, main_execution_context: boolean = false) {
-		const variable = this.component.var_lookup.get(name);
+		return renderer_invalidate(this, name, value, main_execution_context);
-		const member = this.context_lookup.get(name);
-
-		if (variable && (variable.subscribable && (variable.reassigned || variable.export_name))) {
-			return main_execution_context
-			  ? x`${`$$subscribe_${name}`}(${value || name})`
-			  : x`${`$$subscribe_${name}`}($$invalidate(${member.index}, ${value || name}))`;
-		}
-
-		if (name[0] === '$' && name[1] !== '$') {
-			return x`${name.slice(1)}.set(${value || name})`;
-		}
-
-		if (
-			variable && (
-				variable.module || (
-					!variable.referenced &&
-					!variable.is_reactive_dependency &&
-					!variable.export_name &&
-					!name.startsWith('$$')
-				)
-			)
-		) {
-			return value || name;
-		}
-
-		if (value) {
-			return x`$$invalidate(${member.index}, ${value})`;
-		}
-
-		// if this is a reactive declaration, invalidate dependencies recursively
-		const deps = new Set([name]);
-
-		deps.forEach(name => {
-			const reactive_declarations = this.component.reactive_declarations.filter(x =>
-				x.assignees.has(name)
-			);
-			reactive_declarations.forEach(declaration => {
-				declaration.dependencies.forEach(name => {
-					deps.add(name);
-				});
-			});
-		});
-
-		// TODO ideally globals etc wouldn't be here in the first place
-		const filtered = Array.from(deps).filter(n => this.context_lookup.has(n));
-		if (!filtered.length) return null;
-
-		return filtered
-			.map(n => x`$$invalidate(${this.context_lookup.get(n).index}, ${n})`)
-			.reduce((lhs, rhs) => x`${lhs}, ${rhs}`);
 	}
 
 	dirty(names: string[], is_reactive_declaration = false): Expression {

src/compiler/compile/render_dom/invalidate.ts

@@ -33,7 +33,7 @@ export function invalidate(renderer: Renderer, scope: Scope, node: Node, names:
 		if (main_execution_context && !variable.subscribable && variable.name[0] !== '$') {
 			return node;
 		}
-		return renderer.invalidate(variable.name, undefined, main_execution_context);
+		return renderer_invalidate(renderer, variable.name, undefined, main_execution_context);
 	}
 
 	if (!head) {
@@ -79,3 +79,66 @@ export function invalidate(renderer: Renderer, scope: Scope, node: Node, names:
 
 	return invalidate;
 }
+
+export function renderer_invalidate(renderer: Renderer, name: string, value?, main_execution_context: boolean = false) {
+	const variable = renderer.component.var_lookup.get(name);
+
+	if (variable && (variable.subscribable && (variable.reassigned || variable.export_name))) {
+		if (main_execution_context) {
+			return x`${`$$subscribe_${name}`}(${value || name})`;
+		} else {
+			const member = renderer.context_lookup.get(name);
+			return x`${`$$subscribe_${name}`}($$invalidate(${member.index}, ${value || name}))`;
+		}
+	}
+
+	if (name[0] === '$' && name[1] !== '$') {
+		return x`${name.slice(1)}.set(${value || name})`;
+	}
+
+	if (
+		variable && (
+			variable.module || (
+				!variable.referenced &&
+				!variable.is_reactive_dependency &&
+				!variable.export_name &&
+				!name.startsWith('$$')
+			)
+		)
+	) {
+		return value || name;
+	}
+
+	if (value) {
+		if (main_execution_context) {
+			return x`${value}`;
+		} else {
+			const member = renderer.context_lookup.get(name);
+			return x`$$invalidate(${member.index}, ${value})`;
+		}
+	}
+
+	if (main_execution_context) return;
+
+	// if this is a reactive declaration, invalidate dependencies recursively
+	const deps = new Set([name]);
+
+	deps.forEach(name => {
+		const reactive_declarations = renderer.component.reactive_declarations.filter(x =>
+			x.assignees.has(name)
+		);
+		reactive_declarations.forEach(declaration => {
+			declaration.dependencies.forEach(name => {
+				deps.add(name);
+			});
+		});
+	});
+
+	// TODO ideally globals etc wouldn't be here in the first place
+	const filtered = Array.from(deps).filter(n => renderer.context_lookup.has(n));
+	if (!filtered.length) return null;
+
+	return filtered
+		.map(n => x`$$invalidate(${renderer.context_lookup.get(n).index}, ${n})`)
+		.reduce((lhs, rhs) => x`${lhs}, ${rhs}`);
+}

src/compiler/compile/render_dom/wrappers/IfBlock.ts

@@ -448,7 +448,7 @@ export default class IfBlockWrapper extends Wrapper {
 					${name} = ${if_blocks}[${current_block_type_index}] = ${if_block_creators}[${current_block_type_index}](#ctx);
 					${name}.c();
 				} else {
-					${name}.p(#ctx, #dirty);
+					${dynamic && b`${name}.p(#ctx, #dirty);`}
 				}
 				${has_transitions && b`@transition_in(${name}, 1);`}
 				${name}.m(${update_mount_node}, ${anchor});
@@ -472,10 +472,13 @@ export default class IfBlockWrapper extends Wrapper {
 					}
 				`;
 
-			if (dynamic) {
 			block.chunks.update.push(b`
 				let ${previous_block_index} = ${current_block_type_index};
 				${current_block_type_index} = ${select_block_type}(#ctx, #dirty);
+			`);
+
+			if (dynamic) {
+				block.chunks.update.push(b`
 					if (${current_block_type_index} === ${previous_block_index}) {
 						${if_current_block_type_index(b`${if_blocks}[${current_block_type_index}].p(#ctx, #dirty);`)}
 					} else {
@@ -484,8 +487,6 @@ export default class IfBlockWrapper extends Wrapper {
 				`);
 			} else {
 				block.chunks.update.push(b`
-					let ${previous_block_index} = ${current_block_type_index};
-					${current_block_type_index} = ${select_block_type}(#ctx, #dirty);
 					if (${current_block_type_index} !== ${previous_block_index}) {
 						${change_block}
 					}

src/compiler/compile/render_ssr/index.ts

@@ -6,6 +6,9 @@ import Renderer from './Renderer';
 import { INode as TemplateNode } from '../nodes/interfaces'; // TODO
 import Text from '../nodes/Text';
 import { LabeledStatement, Statement, Node } from 'estree';
+import { walk } from 'estree-walker';
+import { extract_names } from 'periscopic';
+import { invalidate } from '../render_dom/invalidate';
 
 export default function ssr(
 	component: Component,
@@ -38,24 +41,94 @@ export default function ssr(
 	const slots = uses_slots ? b`let $$slots = @compute_slots(#slots);` : null;
 
 	const reactive_stores = component.vars.filter(variable => variable.name[0] === '$' && variable.name[1] !== '$');
-	const reactive_store_values = reactive_stores
+	const reactive_store_subscriptions = reactive_stores
+		.filter(store => {
+			const variable = component.var_lookup.get(store.name.slice(1));
+			return !variable || variable.hoistable;
+		})
+		.map(({ name }) => {
+			const store_name = name.slice(1);
+			return b`
+				${component.compile_options.dev && b`@validate_store(${store_name}, '${store_name}');`}
+				${`$$unsubscribe_${store_name}`} = @subscribe(${store_name}, #value => ${name} = #value)
+				${store_name}.subscribe($$value => ${name} = $$value);
+			`;
+		});
+	const reactive_store_unsubscriptions = reactive_stores.map(
+		({ name }) => b`${`$$unsubscribe_${name.slice(1)}`}()`
+	);
+
+	const reactive_store_declarations = reactive_stores
 		.map(({ name }) => {
 			const store_name = name.slice(1);
 			const store = component.var_lookup.get(store_name);
-			if (store && store.hoistable) return null;
 
-			const assignment = b`${name} = @get_store_value(${store_name});`;
+			if (store && store.reassigned) {
+				const unsubscribe = `$$unsubscribe_${store_name}`;
+				const subscribe = `$$subscribe_${store_name}`;
 
-			return component.compile_options.dev
+				return b`let ${name}, ${unsubscribe} = @noop, ${subscribe} = () => (${unsubscribe}(), ${unsubscribe} = @subscribe(${store_name}, $$value => ${name} = $$value), ${store_name})`;
-				? b`@validate_store(${store_name}, '${store_name}'); ${assignment}`
+			}
-				: assignment;
+			return b`let ${name}, ${`$$unsubscribe_${store_name}`};`;
-		})
+		});
-		.filter(Boolean);
 
-	component.rewrite_props(({ name }) => {
+	// instrument get/set store value
+	if (component.ast.instance) {
+		let scope = component.instance_scope;
+		const map = component.instance_scope_map;
+
+		walk(component.ast.instance.content, {
+			enter(node: Node) {
+				if (map.has(node)) {
+					scope = map.get(node);
+				}
+			},
+			leave(node: Node) {
+				if (map.has(node)) {
+					scope = scope.parent;
+				}
+
+				if (node.type === 'AssignmentExpression' || node.type === 'UpdateExpression') {
+					const assignee = node.type === 'AssignmentExpression' ? node.left : node.argument;
+					const names = new Set(extract_names(assignee));
+					const to_invalidate = new Set<string>();
+
+					for (const name of names) {
+						const variable = component.var_lookup.get(name);
+						if (variable &&
+							!variable.hoistable &&
+							!variable.global &&
+							!variable.module &&
+							(
+								variable.subscribable || variable.name[0] === '$'
+							)) {
+								to_invalidate.add(variable.name);
+							}
+					}
+
+					if (to_invalidate.size) {
+						this.replace(
+							invalidate(
+								{ component } as any,
+								scope,
+								node,
+								to_invalidate,
+								true
+							)
+						);
+					}
+				}
+			}
+		});
+	}
+
+	component.rewrite_props(({ name, reassigned }) => {
 		const value = `$${name}`;
 
-		let insert = b`${value} = @get_store_value(${name})`;
+		let insert = reassigned
+			? b`${`$$subscribe_${name}`}()`
+			: b`${`$$unsubscribe_${name}`} = @subscribe(${name}, #value => $${value} = #value)`;
+
 		if (component.compile_options.dev) {
 			insert = b`@validate_store(${name}, '${name}'); ${insert}`;
 		}
@@ -99,35 +172,28 @@ export default function ssr(
 			do {
 				$$settled = true;
 
-				${reactive_store_values}
-
 				${reactive_declarations}
 
 				$$rendered = ${literal};
 			} while (!$$settled);
 
+			${reactive_store_unsubscriptions}
+
 			return $$rendered;
 		`
 		: b`
-			${reactive_store_values}
-
 			${reactive_declarations}
 
+			${reactive_store_unsubscriptions}
+
 			return ${literal};`;
 
 	const blocks = [
 		...injected.map(name => b`let ${name};`),
 		rest,
 		slots,
-		...reactive_stores.map(({ name }) => {
+		...reactive_store_declarations,
-			const store_name = name.slice(1);
+		...reactive_store_subscriptions,
-			const store = component.var_lookup.get(store_name);
-			if (store && store.hoistable) {
-				return b`let ${name} = @get_store_value(${store_name});`;
-			}
-			return b`let ${name};`;
-		}),
-
 		instance_javascript,
 		...parent_bindings,
 		css.code && b`$$result.css.add(#css);`,

src/runtime/internal/await_block.ts

@@ -28,7 +28,9 @@ export function handle_promise(promise, info) {
 					if (i !== index && block) {
 						group_outros();
 						transition_out(block, 1, 1, () => {
+							if (info.blocks[i] === block) {
 								info.blocks[i] = null;
+							}
 						});
 						check_outros();
 					}

test/runtime/samples/reactive-assignment-in-complex-declaration-with-store-2/_config.js

@@ -1,6 +1,5 @@
 // destructure to store value
 export default {
-	skip_if_ssr: true, // pending https://github.com/sveltejs/svelte/issues/3582
 	html: '<h1>2 2 xxx 5 6 9 10 2</h1>',
 	async test({ assert, target, component }) {
 		await component.update();

test/runtime/samples/reactive-assignment-in-complex-declaration-with-store/_config.js

@@ -1,7 +1,6 @@
 // destructure to store
 export default {
 	html: '<h1>2 2 xxx 5 6 9 10 2</h1>',
-	skip_if_ssr: true,
 	async test({ assert, target, component }) {
 		await component.update();
 		assert.htmlEqual(target.innerHTML, '<h1>11 11 yyy 12 13 14 15 11</h1>');

test/runtime/samples/store-assignment-updates-property/_config.js

@@ -5,7 +5,6 @@ export default {
 		<button></button>
 		<button></button>
 	`,
-	skip_if_ssr: true,
 
 	async test({ assert, component, target, window }) {
 		const [btn1, btn2] = target.querySelectorAll('button');

test/runtime/samples/store-auto-resubscribe-immediate/_config.js

@@ -0,0 +1,3 @@
+export default {
+	html: '{"answer":4}'
+};

test/runtime/samples/store-auto-resubscribe-immediate/main.svelte

@@ -0,0 +1,31 @@
+<script>
+	import { writable } from '../../../../store';
+
+	let value = writable({ foo: 1, bar: 2 });
+	$value.foo = $value.foo + $value.bar; // 3
+	$value.bar = $value.foo * $value.bar; // 6
+
+	// should resubscribe immediately
+	value = writable({ foo: $value.foo + 2, bar: $value.bar - 2 }); // { foo: 5, bar: 4 }
+
+	// should mutate the store value
+	$value.baz = $value.foo + $value.bar; // { foo: 5, bar: 4, baz: 9 }
+
+	// should resubscribe immediately
+	value = writable({ qux: $value.baz - $value.foo }); // { qux: 4 }
+
+	// making sure instrumentation returns the expression value
+	$value = {
+		one: writable(
+			$value = {
+				two: ({ $value } = { $value: { fred: $value.qux } }) // { fred: 4 }
+			} // { two: { $value: { fred: 4 } } }
+		) // { one: { two: { $value: { fred: 4 } } } }
+	};
+
+	const one = $value.one;
+
+	value.update(val => ({ answer: $one.two.$value.fred })); // { answer: 4 }
+</script>
+
+{JSON.stringify($value)}

test/runtime/samples/transition-js-await-block-outros/_config.js

@@ -0,0 +1,174 @@
+let fulfil;
+
+export default {
+	props: {
+		promise: new Promise((f) => {
+			fulfil = f;
+		})
+	},
+	intro: true,
+
+	async test({ assert, target, component, raf }) {
+		assert.htmlEqual(target.innerHTML, '<p class="pending" foo="0.0">loading...</p>');
+
+		let time = 0;
+
+		raf.tick(time += 50);
+		assert.htmlEqual(target.innerHTML, '<p class="pending" foo="0.5">loading...</p>');
+
+		await fulfil(42);
+
+		assert.htmlEqual(target.innerHTML, `
+			<p class="then" foo="0.0">42</p>
+			<p class="pending" foo="0.5">loading...</p>
+		`);
+
+		// see the transition 30% complete
+		raf.tick(time += 30);
+		assert.htmlEqual(target.innerHTML, `
+			<p class="then" foo="0.3">42</p>
+			<p class="pending" foo="0.2">loading...</p>
+		`);
+
+		// completely transition in the {:then} block
+		raf.tick(time += 70);
+		assert.htmlEqual(target.innerHTML, `
+			<p class="then" foo="1.0">42</p>
+		`);
+
+		// update promise #1
+		component.promise = new Promise((f) => {
+			fulfil = f;
+		});
+		await Promise.resolve();
+
+		assert.htmlEqual(target.innerHTML, `
+			<p class="then" foo="1.0">42</p>
+			<p class="pending" foo="0.0">loading...</p>
+		`);
+
+		raf.tick(time += 100);
+
+		assert.htmlEqual(target.innerHTML, `
+			<p class="pending" foo="1.0">loading...</p>
+		`);
+
+		await fulfil(43);
+		assert.htmlEqual(target.innerHTML, `
+			<p class="pending" foo="1.0">loading...</p>
+			<p class="then" foo="0.0">43</p>
+		`);
+
+		raf.tick(time += 100);
+		assert.htmlEqual(target.innerHTML, `
+			<p class="then" foo="1.0">43</p>
+		`);
+
+		// update promise #2
+		component.promise = new Promise((f) => {
+			fulfil = f;
+		});
+		await Promise.resolve();
+
+		assert.htmlEqual(target.innerHTML, `
+			<p class="then" foo="1.0">43</p>
+			<p class="pending" foo="0.0">loading...</p>
+		`);
+
+		raf.tick(time += 50);
+
+		assert.htmlEqual(target.innerHTML, `
+			<p class="then" foo="0.5">43</p>
+			<p class="pending" foo="0.5">loading...</p>
+		`);
+
+		await fulfil(44);
+		assert.htmlEqual(target.innerHTML, `
+			<p class="then" foo="0.5">43</p>
+			<p class="pending" foo="0.5">loading...</p>
+			<p class="then" foo="0.0">44</p>
+		`);
+
+		raf.tick(time += 100);
+		assert.htmlEqual(target.innerHTML, `
+			<p class="then" foo="1.0">44</p>
+		`);
+
+		// update promise #3 - quick succession
+		component.promise = new Promise((f) => {
+			fulfil = f;
+		});
+		await Promise.resolve();
+		assert.htmlEqual(target.innerHTML, `
+			<p class="then" foo="1.0">44</p>
+			<p class="pending" foo="0.0">loading...</p>
+		`);
+
+		raf.tick(time += 40);
+		assert.htmlEqual(target.innerHTML, `
+			<p class="then" foo="0.6">44</p>
+			<p class="pending" foo="0.4">loading...</p>
+		`);
+
+		await fulfil(45);
+
+		assert.htmlEqual(target.innerHTML, `
+			<p class="then" foo="0.6">44</p>
+			<p class="pending" foo="0.4">loading...</p>
+			<p class="then" foo="0.0">45</p>
+		`);
+
+		raf.tick(time += 20);
+		assert.htmlEqual(target.innerHTML, `
+			<p class="then" foo="0.4">44</p>
+			<p class="pending" foo="0.2">loading...</p>
+			<p class="then" foo="0.2">45</p>
+		`);
+
+		component.promise = new Promise((f) => {
+			fulfil = f;
+		});
+		await Promise.resolve();
+
+		assert.htmlEqual(target.innerHTML, `
+			<p class="then" foo="0.4">44</p>
+			<p class="pending" foo="0.2">loading...</p>
+			<p class="then" foo="0.2">45</p>
+			<p class="pending" foo="0.0">loading...</p>
+		`);
+
+		raf.tick(time += 10);
+		assert.htmlEqual(target.innerHTML, `
+			<p class="then" foo="0.3">44</p>
+			<p class="pending" foo="0.1">loading...</p>
+			<p class="then" foo="0.1">45</p>
+			<p class="pending" foo="0.1">loading...</p>
+		`);
+
+		await fulfil(46);
+
+		assert.htmlEqual(target.innerHTML, `
+			<p class="then" foo="0.3">44</p>
+			<p class="pending" foo="0.1">loading...</p>
+			<p class="then" foo="0.1">45</p>
+			<p class="pending" foo="0.1">loading...</p>
+			<p class="then" foo="0.0">46</p>
+		`);
+
+		raf.tick(time += 10);
+		assert.htmlEqual(target.innerHTML, `
+			<p class="then" foo="0.2">44</p>
+			<p class="then" foo="0.1">46</p>
+		`);
+
+		raf.tick(time += 20);
+		assert.htmlEqual(target.innerHTML, `
+			<p class="then" foo="0.3">46</p>
+		`);
+
+		raf.tick(time += 70);
+		assert.htmlEqual(target.innerHTML, `
+			<p class="then" foo="1.0">46</p>
+		`);
+	}
+};

test/runtime/samples/transition-js-await-block-outros/main.svelte

@@ -0,0 +1,20 @@
+<script>
+	export let promise;
+
+	function foo(node) {
+		return {
+			duration: 100,
+			tick: t => {
+				node.setAttribute('foo', t.toFixed(1));
+			}
+		};
+	}
+</script>
+
+{#await promise}
+	<p class='pending' transition:foo>loading...</p>
+{:then value}
+	<p class='then' transition:foo>{value}</p>
+{:catch error}
+	<p class='catch' transition:foo>{error.message}</p>
+{/await}

test/runtime/samples/transition-js-if-else-block-not-dynamic-outro/_config.js

@@ -0,0 +1,38 @@
+export default {
+	async test({ assert, component, target, window, raf }) {
+		const t = target.querySelector('#t');
+
+		await (component.condition = false);
+
+		let time = 0;
+		raf.tick(time += 25);
+
+		assert.htmlEqual(target.innerHTML, `
+			<div id="t" foo="0.75">TRUE</div>
+			<div id="f">FALSE</div>
+		`);
+
+		// toggling back in the middle of the out transition
+		// will reuse the previous element
+		await (component.condition = true);
+
+		assert.htmlEqual(target.innerHTML, `
+			<div id="f">FALSE</div>
+			<div id="t" foo="1">TRUE</div>
+		`);
+		assert.equal(target.querySelector('#t'), t);
+
+		raf.tick(time += 25);
+
+		assert.htmlEqual(target.innerHTML, `
+			<div id="f" foo="0.75">FALSE</div>
+			<div id="t" foo="1">TRUE</div>
+		`);
+
+		raf.tick(time += 75);
+
+		assert.htmlEqual(target.innerHTML, `
+			<div id="t" foo="1">TRUE</div>
+		`);
+	}
+};

test/runtime/samples/transition-js-if-else-block-not-dynamic-outro/main.svelte

@@ -0,0 +1,18 @@
+<script>
+	export let condition = true;
+
+	function foo(node) {
+		return {
+			duration: 100,
+			tick: t => {
+				node.setAttribute('foo', t);
+			}
+		};
+	}
+</script>
+
+{#if condition}
+	<div id="t" out:foo>TRUE</div>
+{:else}
+	<div id="f" out:foo>FALSE</div>
+{/if}