fix: correctly add `__svelte_meta` after else-if chains (#17830)

While looking into something else I spotted the fact that we use `false`
to indicate 'else' in an `{#if ...}` block (whether there's an `else`
block to render or not). If we instead use `-1`, and use `<!--[0-->` to
indicate that the first block in an if-elseif chain was rendered...

- instead of `<!--[-->`, we do `<!--[0-->`
- instead of `<!--[!-->`, we do `<!--[-1-->`
- all others stay the same

...we can simplify things a bit — the `key` argument to `update_branch`
is always a number (which probably has some microscopic benefits in
terms of making it monomorphic when `else` is defined, and less
polymorphic when it isn't), and the hydration mismatch code only needs
to consider one type of hydration marker.

In the process, I discovered a bug — the dev-time `add_locations`
function fails on hydration markers like `<!--[1-->`. This PR fixes it.
pull/17842/head
Rich Harris 2 months ago committed by GitHub
parent 7717ba01b4
commit 86ec210866
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: correctly add \_\_svelte_meta after else-if chains

@ -51,7 +51,7 @@ export function IfBlock(node, context) {
}
}
const render_call = b.stmt(b.call('$$render', consequent_id, index > 0 && b.literal(index)));
const render_call = b.stmt(b.call('$$render', consequent_id, index !== 0 && b.literal(index)));
const new_if = b.if(test, render_call);
if (last_if) {
@ -71,7 +71,7 @@ export function IfBlock(node, context) {
const alternate_id = b.id(context.state.scope.generate('alternate'));
statements.push(b.var(alternate_id, b.arrow([b.id('$$anchor')], alternate)));
last_if.alternate = b.stmt(b.call('$$render', alternate_id, b.literal(false)));
last_if.alternate = b.stmt(b.call('$$render', alternate_id, b.literal(-1)));
}
// Build $.if() arguments

@ -1,8 +1,8 @@
/** @import { BlockStatement, Expression, IfStatement, Statement } from 'estree' */
/** @import { BlockStatement, Expression, IfStatement } from 'estree' */
/** @import { AST } from '#compiler' */
/** @import { ComponentContext } from '../types.js' */
import * as b from '#compiler/builders';
import { block_close, block_open, block_open_else, create_child_block } from './shared/utils.js';
import { block_close, create_child_block } from './shared/utils.js';
/**
* @param {AST.IfBlock} node
@ -10,7 +10,7 @@ import { block_close, block_open, block_open_else, create_child_block } from './
*/
export function IfBlock(node, context) {
const consequent = /** @type {BlockStatement} */ (context.visit(node.consequent));
consequent.body.unshift(b.stmt(b.call(b.id('$$renderer.push'), block_open)));
consequent.body.unshift(b.stmt(b.call(b.id('$$renderer.push'), b.literal(`<!--[0-->`))));
/** @type {IfStatement} */
let if_statement = b.if(/** @type {Expression} */ (context.visit(node.test)), consequent);
@ -34,7 +34,7 @@ export function IfBlock(node, context) {
// Handle final else (or remaining async chain)
const final_alternate = alt ? /** @type {BlockStatement} */ (context.visit(alt)) : b.block([]);
final_alternate.body.unshift(b.stmt(b.call(b.id('$$renderer.push'), block_open_else)));
final_alternate.body.unshift(b.stmt(b.call(b.id('$$renderer.push'), b.literal(`<!--[-1-->`))));
current_if.alternate = final_alternate;
context.state.template.push(

@ -1,6 +1,6 @@
/** @import { SourceLocation } from '#client' */
import { COMMENT_NODE, DOCUMENT_FRAGMENT_NODE, ELEMENT_NODE } from '#client/constants';
import { HYDRATION_END, HYDRATION_START, HYDRATION_START_ELSE } from '../../../constants.js';
import { HYDRATION_END, HYDRATION_START } from '../../../constants.js';
import { hydrating } from '../dom/hydration.js';
import { dev_stack } from '../context.js';
@ -50,7 +50,7 @@ function assign_locations(node, filename, locations) {
while (node && i < locations.length) {
if (hydrating && node.nodeType === COMMENT_NODE) {
var comment = /** @type {Comment} */ (node);
if (comment.data === HYDRATION_START || comment.data === HYDRATION_START_ELSE) depth += 1;
if (comment.data[0] === HYDRATION_START) depth += 1;
else if (comment.data[0] === HYDRATION_END) depth -= 1;
}

@ -11,7 +11,6 @@ import {
} from '../hydration.js';
import { block } from '../../reactivity/effects.js';
import { BranchManager } from './branches.js';
import { HYDRATION_START, HYDRATION_START_ELSE } from '../../../../constants.js';
/**
* @param {TemplateNode} node
@ -37,21 +36,9 @@ export function if_block(node, fn, elseif = false) {
function update_branch(key, fn) {
if (hydrating) {
var data = read_hydration_instruction(/** @type {TemplateNode} */ (marker));
/**
* @type {number | false}
* "[" = branch 0, "[1" = branch 1, "[2" = branch 2, ..., "[!" = else (false)
*/
var hydrated_key;
if (data === HYDRATION_START) {
hydrated_key = 0;
} else if (data === HYDRATION_START_ELSE) {
hydrated_key = false;
} else {
hydrated_key = parseInt(data.substring(1)); // "[1", "[2", etc.
}
if (key !== hydrated_key) {
// "[n" = branch n, "[-1" = else
if (key !== parseInt(data.substring(1))) {
// Hydration mismatch: remove everything inside the anchor and start fresh.
// This could happen with `{#if browser}...{/if}`, for example
var anchor = skip_nodes();
@ -79,7 +66,7 @@ export function if_block(node, fn, elseif = false) {
});
if (!has_branch) {
update_branch(false, null);
update_branch(-1, null);
}
}, flags);
}

@ -0,0 +1,35 @@
import { flushSync } from 'svelte';
import { test } from '../../test';
export default test({
compileOptions: {
dev: true
},
html: `<p>before</p><p>during</p><p>after</p>`,
async test({ target, assert }) {
const ps = target.querySelectorAll('p');
// @ts-expect-error
assert.deepEqual(ps[0].__svelte_meta.loc, {
file: 'main.svelte',
line: 1,
column: 0
});
// @ts-expect-error
assert.deepEqual(ps[1].__svelte_meta.loc, {
file: 'main.svelte',
line: 6,
column: 1
});
// @ts-expect-error
assert.deepEqual(ps[2].__svelte_meta.loc, {
file: 'main.svelte',
line: 11,
column: 0
});
}
});

@ -0,0 +1,11 @@
<p>before</p>
{#if false}
<p>during</p>
{:else if true}
<p>during</p>
{:else if false}
<p>during</p>
{/if}
<p>after</p>

@ -3,7 +3,7 @@ import * as $ from 'svelte/internal/server';
export default function Async_const($$renderer) {
if (true) {
$$renderer.push('<!--[-->');
$$renderer.push('<!--[0-->');
let a;
let b;
@ -22,7 +22,7 @@ export default function Async_const($$renderer) {
$$renderer.async([promises[1]], ($$renderer) => $$renderer.push(() => $.escape(b)));
$$renderer.push(`</p>`);
} else {
$$renderer.push('<!--[!-->');
$$renderer.push('<!--[-1-->');
}
$$renderer.push(`<!--]-->`);

@ -22,7 +22,7 @@ export default function Async_if_alternate_hoisting($$anchor) {
};
$.if(node, ($$render) => {
if ($.get($$condition)) $$render(consequent); else $$render(alternate, false);
if ($.get($$condition)) $$render(consequent); else $$render(alternate, -1);
});
});

@ -4,10 +4,10 @@ import * as $ from 'svelte/internal/server';
export default function Async_if_alternate_hoisting($$renderer) {
$$renderer.child_block(async ($$renderer) => {
if ((await $.save(Promise.resolve(false)))()) {
$$renderer.push('<!--[-->');
$$renderer.push('<!--[0-->');
$$renderer.push(async () => $.escape(await Promise.reject('no no no')));
} else {
$$renderer.push('<!--[!-->');
$$renderer.push('<!--[-1-->');
$$renderer.push(async () => $.escape(await Promise.resolve('yes yes yes')));
}
});

@ -35,7 +35,7 @@ export default function Async_if_chain($$anchor) {
};
$.if(node, ($$render) => {
if (foo) $$render(consequent); else if (bar) $$render(consequent_1, 1); else $$render(alternate, false);
if (foo) $$render(consequent); else if (bar) $$render(consequent_1, 1); else $$render(alternate, -1);
});
});
@ -74,7 +74,7 @@ export default function Async_if_chain($$anchor) {
$.if(
node_2,
($$render) => {
if ($.get($$condition)) $$render(consequent_4); else $$render(alternate_1, false);
if ($.get($$condition)) $$render(consequent_4); else $$render(alternate_1, -1);
},
true
);
@ -84,7 +84,7 @@ export default function Async_if_chain($$anchor) {
};
$.if(node_1, ($$render) => {
if ($.get($$condition)) $$render(consequent_2); else if (bar) $$render(consequent_3, 1); else $$render(alternate_2, false);
if ($.get($$condition)) $$render(consequent_2); else if (bar) $$render(consequent_3, 1); else $$render(alternate_2, -1);
});
});
@ -123,7 +123,7 @@ export default function Async_if_chain($$anchor) {
$.if(
node_4,
($$render) => {
if ($.get($$condition)) $$render(consequent_7); else $$render(alternate_3, false);
if ($.get($$condition)) $$render(consequent_7); else $$render(alternate_3, -1);
},
true
);
@ -133,7 +133,7 @@ export default function Async_if_chain($$anchor) {
};
$.if(node_3, ($$render) => {
if ($.get($$condition)) $$render(consequent_5); else if (bar) $$render(consequent_6, 1); else $$render(alternate_4, false);
if ($.get($$condition)) $$render(consequent_5); else if (bar) $$render(consequent_6, 1); else $$render(alternate_4, -1);
});
});
@ -167,7 +167,7 @@ export default function Async_if_chain($$anchor) {
};
$.if(node_5, ($$render) => {
if (simple1) $$render(consequent_8); else if (simple2 > 10) $$render(consequent_9, 1); else if ($.get(d)) $$render(consequent_10, 2); else $$render(alternate_5, false);
if (simple1) $$render(consequent_8); else if (simple2 > 10) $$render(consequent_9, 1); else if ($.get(d)) $$render(consequent_10, 2); else $$render(alternate_5, -1);
});
}
@ -193,7 +193,7 @@ export default function Async_if_chain($$anchor) {
};
$.if(node_6, ($$render) => {
if ($.get(blocking) > 10) $$render(consequent_11); else if ($.get(blocking) > 5) $$render(consequent_12, 1); else $$render(alternate_6, false);
if ($.get(blocking) > 10) $$render(consequent_11); else if ($.get(blocking) > 5) $$render(consequent_12, 1); else $$render(alternate_6, -1);
});
});

@ -12,13 +12,13 @@ export default function Async_if_chain($$renderer) {
$$renderer.async_block([$$promises[0]], ($$renderer) => {
if (foo) {
$$renderer.push('<!--[-->');
$$renderer.push('<!--[0-->');
$$renderer.push(`foo`);
} else if (bar) {
$$renderer.push('<!--[1-->');
$$renderer.push(`bar`);
} else {
$$renderer.push('<!--[!-->');
$$renderer.push('<!--[-1-->');
$$renderer.push(`else`);
}
});
@ -27,20 +27,20 @@ export default function Async_if_chain($$renderer) {
$$renderer.async_block([$$promises[0]], async ($$renderer) => {
if ((await $.save(foo))()) {
$$renderer.push('<!--[-->');
$$renderer.push('<!--[0-->');
$$renderer.push(`foo`);
} else if (bar) {
$$renderer.push('<!--[1-->');
$$renderer.push(`bar`);
} else {
$$renderer.push('<!--[!-->');
$$renderer.push('<!--[-1-->');
$$renderer.child_block(async ($$renderer) => {
if ((await $.save(baz))()) {
$$renderer.push('<!--[-->');
$$renderer.push('<!--[0-->');
$$renderer.push(`baz`);
} else {
$$renderer.push('<!--[!-->');
$$renderer.push('<!--[-1-->');
$$renderer.push(`else`);
}
});
@ -53,20 +53,20 @@ export default function Async_if_chain($$renderer) {
$$renderer.async_block([$$promises[0]], async ($$renderer) => {
if ((await $.save(foo))() > 10) {
$$renderer.push('<!--[-->');
$$renderer.push('<!--[0-->');
$$renderer.push(`foo`);
} else if (bar) {
$$renderer.push('<!--[1-->');
$$renderer.push(`bar`);
} else {
$$renderer.push('<!--[!-->');
$$renderer.push('<!--[-1-->');
$$renderer.async_block([$$promises[0]], async ($$renderer) => {
if ((await $.save(foo))() > 5) {
$$renderer.push('<!--[-->');
$$renderer.push('<!--[0-->');
$$renderer.push(`baz`);
} else {
$$renderer.push('<!--[!-->');
$$renderer.push('<!--[-1-->');
$$renderer.push(`else`);
}
});
@ -78,7 +78,7 @@ export default function Async_if_chain($$renderer) {
$$renderer.push(`<!--]--> `);
if (simple1) {
$$renderer.push('<!--[-->');
$$renderer.push('<!--[0-->');
$$renderer.push(`foo`);
} else if (simple2 > 10) {
$$renderer.push('<!--[1-->');
@ -87,7 +87,7 @@ export default function Async_if_chain($$renderer) {
$$renderer.push('<!--[2-->');
$$renderer.push(`baz`);
} else {
$$renderer.push('<!--[!-->');
$$renderer.push('<!--[-1-->');
$$renderer.push(`else`);
}
@ -95,13 +95,13 @@ export default function Async_if_chain($$renderer) {
$$renderer.async_block([$$promises[0]], ($$renderer) => {
if (blocking() > 10) {
$$renderer.push('<!--[-->');
$$renderer.push('<!--[0-->');
$$renderer.push(`foo`);
} else if (blocking() > 5) {
$$renderer.push('<!--[1-->');
$$renderer.push(`bar`);
} else {
$$renderer.push('<!--[!-->');
$$renderer.push('<!--[-1-->');
$$renderer.push(`else`);
}
});

@ -22,7 +22,7 @@ export default function Async_if_hoisting($$anchor) {
};
$.if(node, ($$render) => {
if ($.get($$condition)) $$render(consequent); else $$render(alternate, false);
if ($.get($$condition)) $$render(consequent); else $$render(alternate, -1);
});
});

@ -4,10 +4,10 @@ import * as $ from 'svelte/internal/server';
export default function Async_if_hoisting($$renderer) {
$$renderer.child_block(async ($$renderer) => {
if ((await $.save(Promise.resolve(true)))()) {
$$renderer.push('<!--[-->');
$$renderer.push('<!--[0-->');
$$renderer.push(async () => $.escape(await Promise.resolve('yes yes yes')));
} else {
$$renderer.push('<!--[!-->');
$$renderer.push('<!--[-1-->');
$$renderer.push(async () => $.escape(await Promise.reject('no no no')));
}
});

@ -18,7 +18,7 @@ export default function Async_in_derived($$renderer, $$props) {
]);
if (true) {
$$renderer.push('<!--[-->');
$$renderer.push('<!--[0-->');
let yes1;
let yes2;
@ -47,7 +47,7 @@ export default function Async_in_derived($$renderer, $$props) {
}
]);
} else {
$$renderer.push('<!--[!-->');
$$renderer.push('<!--[-1-->');
}
$$renderer.push(`<!--]-->`);

@ -57,13 +57,13 @@ export default function Select_with_rich_content($$renderer) {
$$renderer.push(`<!--]--></select> <select>`);
if (show) {
$$renderer.push('<!--[-->');
$$renderer.push('<!--[0-->');
$$renderer.option({}, ($$renderer) => {
$$renderer.push(`Visible`);
});
} else {
$$renderer.push('<!--[!-->');
$$renderer.push('<!--[-1-->');
}
$$renderer.push(`<!--]--></select> <select><!---->`);
@ -148,7 +148,7 @@ export default function Select_with_rich_content($$renderer) {
$$renderer.push(`<!--]--></select> <select>`);
if (show) {
$$renderer.push('<!--[-->');
$$renderer.push('<!--[0-->');
$$renderer.push(`<!--[-->`);
const each_array_4 = $.ensure_array_like(items);
@ -161,7 +161,7 @@ export default function Select_with_rich_content($$renderer) {
$$renderer.push(`<!--]-->`);
} else {
$$renderer.push('<!--[!-->');
$$renderer.push('<!--[-1-->');
}
$$renderer.push(`<!--]--></select> <select>`);
@ -227,10 +227,10 @@ export default function Select_with_rich_content($$renderer) {
$$renderer.push(`<!--]--><!></select> <select>`);
if (show) {
$$renderer.push('<!--[-->');
$$renderer.push('<!--[0-->');
conditional_option($$renderer);
} else {
$$renderer.push('<!--[!-->');
$$renderer.push('<!--[-1-->');
}
$$renderer.push(`<!--]--><!></select>`);

Loading…
Cancel
Save