Skip to content

fix: store DOM boundaries on effects #12215

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

Merged
merged 40 commits into from
Jun 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
239ccd5
WIP
Rich-Harris Jun 26, 2024
aa553ef
progress
Rich-Harris Jun 27, 2024
7e26c2b
fix
Rich-Harris Jun 27, 2024
8740448
add comment
Rich-Harris Jun 27, 2024
88eb2e1
update tests
Rich-Harris Jun 27, 2024
4218695
mostly fix dynamic elements
Rich-Harris Jun 27, 2024
d81f521
delete unused code
Rich-Harris Jun 27, 2024
15e2870
remove unused code
Rich-Harris Jun 27, 2024
7bafd25
more
Rich-Harris Jun 27, 2024
ea0acce
tidy up
Rich-Harris Jun 27, 2024
1a14f8c
fix
Rich-Harris Jun 27, 2024
27b38cd
more
Rich-Harris Jun 27, 2024
0366bd2
relink effects inside each block
Rich-Harris Jun 27, 2024
af05454
simpler to just leave these in (without children) and ignore them
Rich-Harris Jun 27, 2024
520b3e1
fix head stuff
Rich-Harris Jun 27, 2024
7f26fd0
tidy up
Rich-Harris Jun 27, 2024
a1afbac
fix some type errors
Rich-Harris Jun 27, 2024
760bb1b
simplify
Rich-Harris Jun 27, 2024
ce5d53e
use hydration marker as effect boundary where possible
Rich-Harris Jun 27, 2024
469c73e
all tests passing
Rich-Harris Jun 27, 2024
54e3a2f
tidy up
Rich-Harris Jun 27, 2024
17c95e2
tidy up a bit
Rich-Harris Jun 27, 2024
35dbddd
tidy up
Rich-Harris Jun 27, 2024
e8fb7f1
beat typescript into submission
Rich-Harris Jun 27, 2024
c2f4787
bring tests over from fix-each-dom-bug
Rich-Harris Jun 27, 2024
5d078fa
tweaks
Rich-Harris Jun 27, 2024
06a8f34
simplify a tad
Rich-Harris Jun 27, 2024
3a2a969
tidy up
Rich-Harris Jun 27, 2024
fdf3d84
simplify
Rich-Harris Jun 27, 2024
29ab9b5
reduce indirection
Rich-Harris Jun 27, 2024
6dabf23
belt and braces
Rich-Harris Jun 27, 2024
59f1ecf
tidy
Rich-Harris Jun 27, 2024
4cb822c
revert config change
Rich-Harris Jun 27, 2024
ddcf3ff
missed a spot
Rich-Harris Jun 27, 2024
319300e
regenerate types
Rich-Harris Jun 27, 2024
9f76267
cleaner separation between EachState and EachItem - precursor to effi…
Rich-Harris Jun 28, 2024
83d60e8
Merge branch 'main' into effect-dom-boundaries
Rich-Harris Jun 28, 2024
192e344
HMR fix
Rich-Harris Jun 28, 2024
d4a2611
set effects
Rich-Harris Jun 29, 2024
6ec803d
FINALLY
Rich-Harris Jun 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/svelte/src/compiler/phases/1-parse/state/tag.js
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,10 @@ function special(parser) {
type: 'RenderTag',
start,
end: parser.index,
expression: expression
expression: expression,
metadata: {
dynamic: false
}
});
}
}
7 changes: 7 additions & 0 deletions packages/svelte/src/compiler/phases/2-analyze/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1520,6 +1520,13 @@ const common_visitors = {
return;
}
}
},
Component(node, context) {
const binding = context.state.scope.get(
node.name.includes('.') ? node.name.slice(0, node.name.indexOf('.')) : node.name
);

node.metadata.dynamic = binding !== null && binding.kind !== 'normal';
}
};

Expand Down
6 changes: 5 additions & 1 deletion packages/svelte/src/compiler/phases/2-analyze/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,11 @@ const validation = {
});
},
RenderTag(node, context) {
const callee = unwrap_optional(node.expression).callee;

node.metadata.dynamic =
callee.type !== 'Identifier' || context.state.scope.get(callee.name)?.kind !== 'normal';

context.state.analysis.uses_render_tags = true;

const raw_args = unwrap_optional(node.expression).arguments;
Expand All @@ -642,7 +647,6 @@ const validation = {
}
}

const callee = unwrap_optional(node.expression).callee;
if (
callee.type === 'MemberExpression' &&
callee.property.type === 'Identifier' &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
EACH_KEYED,
is_capture_event,
TEMPLATE_FRAGMENT,
TEMPLATE_UNSET_START,
TEMPLATE_USE_IMPORT_NODE,
TRANSITION_GLOBAL,
TRANSITION_IN,
Expand Down Expand Up @@ -942,6 +943,7 @@ function serialize_inline_component(node, component_name, context) {
fn = (node_id) => {
return b.call(
'$.component',
node_id,
b.thunk(/** @type {import('estree').Expression} */ (context.visit(node.expression))),
b.arrow(
[b.id(component_name)],
Expand Down Expand Up @@ -1680,14 +1682,35 @@ export const template_visitors = {

process_children(trimmed, expression, false, { ...context, state });

var first = trimmed[0];

/**
* If the first item in an effect is a static slot or render tag, it will clone
* a template but without creating a child effect. In these cases, we need to keep
* the current `effect.nodes.start` undefined, so that it can be populated by
* the item in question
* TODO come up with a better name than `unset`
*/
var unset = false;

if (first.type === 'SlotElement') unset = true;
if (first.type === 'RenderTag' && !first.metadata.dynamic) unset = true;
if (first.type === 'Component' && !first.metadata.dynamic && !context.state.options.hmr) {
unset = true;
}

const use_comment_template = state.template.length === 1 && state.template[0] === '<!>';

if (use_comment_template) {
// special case — we can use `$.comment` instead of creating a unique template
body.push(b.var(id, b.call('$.comment')));
body.push(b.var(id, b.call('$.comment', unset && b.literal(unset))));
} else {
let flags = TEMPLATE_FRAGMENT;

if (unset) {
flags |= TEMPLATE_UNSET_START;
}

if (state.metadata.context.template_needs_import_node) {
flags |= TEMPLATE_USE_IMPORT_NODE;
}
Expand Down Expand Up @@ -1832,27 +1855,26 @@ export const template_visitors = {
context.state.template.push('<!>');
const callee = unwrap_optional(node.expression).callee;
const raw_args = unwrap_optional(node.expression).arguments;
const is_reactive =
callee.type !== 'Identifier' || context.state.scope.get(callee.name)?.kind !== 'normal';

/** @type {import('estree').Expression[]} */
const args = [context.state.node];
for (const arg of raw_args) {
args.push(b.thunk(/** @type {import('estree').Expression} */ (context.visit(arg))));
}
const args = raw_args.map((arg) =>
b.thunk(/** @type {import('estree').Expression} */ (context.visit(arg)))
);

let snippet_function = /** @type {import('estree').Expression} */ (context.visit(callee));
if (context.state.options.dev) {
snippet_function = b.call('$.validate_snippet', snippet_function);
}

if (is_reactive) {
context.state.init.push(b.stmt(b.call('$.snippet', b.thunk(snippet_function), ...args)));
if (node.metadata.dynamic) {
context.state.init.push(
b.stmt(b.call('$.snippet', context.state.node, b.thunk(snippet_function), ...args))
);
} else {
context.state.init.push(
b.stmt(
(node.expression.type === 'CallExpression' ? b.call : b.maybe_call)(
snippet_function,
context.state.node,
...args
)
)
Expand Down Expand Up @@ -1915,7 +1937,7 @@ export const template_visitors = {
}

if (node.name === 'noscript') {
context.state.template.push('<!>');
context.state.template.push('<noscript></noscript>');
return;
}
if (node.name === 'script') {
Expand Down Expand Up @@ -2985,16 +3007,14 @@ export const template_visitors = {
}
},
Component(node, context) {
const binding = context.state.scope.get(
node.name.includes('.') ? node.name.slice(0, node.name.indexOf('.')) : node.name
);
if (binding !== null && binding.kind !== 'normal') {
if (node.metadata.dynamic) {
// Handle dynamic references to what seems like static inline components
const component = serialize_inline_component(node, '$$component', context);
context.state.init.push(
b.stmt(
b.call(
'$.component',
context.state.node,
// TODO use untrack here to not update when binding changes?
// Would align with Svelte 4 behavior, but it's arguably nicer/expected to update this
b.thunk(
Expand All @@ -3006,6 +3026,7 @@ export const template_visitors = {
);
return;
}

const component = serialize_inline_component(node, node.name, context);
context.state.init.push(component);
},
Expand Down
6 changes: 6 additions & 0 deletions packages/svelte/src/compiler/types/template.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,9 @@ export interface DebugTag extends BaseNode {
export interface RenderTag extends BaseNode {
type: 'RenderTag';
expression: SimpleCallExpression | (ChainExpression & { expression: SimpleCallExpression });
metadata: {
dynamic: boolean;
};
}

type Tag = ExpressionTag | HtmlTag | ConstTag | DebugTag | RenderTag;
Expand Down Expand Up @@ -271,6 +274,9 @@ interface BaseElement extends BaseNode {

export interface Component extends BaseElement {
type: 'Component';
metadata: {
dynamic: boolean;
};
}

interface TitleElement extends BaseElement {
Expand Down
1 change: 1 addition & 0 deletions packages/svelte/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export const TRANSITION_GLOBAL = 1 << 2;

export const TEMPLATE_FRAGMENT = 1;
export const TEMPLATE_USE_IMPORT_NODE = 1 << 1;
export const TEMPLATE_UNSET_START = 1 << 2;

export const HYDRATION_START = '[';
export const HYDRATION_END = ']';
Expand Down
1 change: 1 addition & 0 deletions packages/svelte/src/internal/client/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export const EFFECT_TRANSPARENT = 1 << 15;
/** Svelte 4 legacy mode props need to be handled with deriveds and be recognized elsewhere, hence the dedicated flag */
export const LEGACY_DERIVED_PROP = 1 << 16;
export const INSPECT_EFFECT = 1 << 17;
export const HEAD_EFFECT = 1 << 18;

export const STATE_SYMBOL = Symbol('$state');
export const STATE_FROZEN_SYMBOL = Symbol('$state.frozen');
Expand Down
2 changes: 1 addition & 1 deletion packages/svelte/src/internal/client/dev/hmr.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export function hmr(source) {
/** @type {import("#client").Effect} */
let effect;

block(() => {
block(anchor, 0, () => {
const component = get(source);

if (effect) {
Expand Down
2 changes: 1 addition & 1 deletion packages/svelte/src/internal/client/dom/blocks/await.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export function await_block(anchor, get_input, pending_fn, then_fn, catch_fn) {
}
}

var effect = block(() => {
var effect = block(anchor, 0, () => {
if (input === (input = get_input())) return;

if (is_promise(input)) {
Expand Down
Loading
Loading