Skip to content

Commit ef206fe

Browse files
trueadmRich-Harris
andauthored
fix: improve derived output for ssr (#10757)
* fix: improve derived output for ssr * ts * Update .changeset/rotten-rules-invite.md --------- Co-authored-by: Rich Harris <[email protected]>
1 parent 6f3fcc0 commit ef206fe

File tree

7 files changed

+156
-71
lines changed

7 files changed

+156
-71
lines changed

.changeset/rotten-rules-invite.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"svelte": patch
3+
---
4+
5+
fix: use getters for derived class state fields, with memoisation

packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ export const global_visitors = {
1717
// rewrite `this.#foo` as `this.#foo.v` inside a constructor
1818
if (node.property.type === 'PrivateIdentifier') {
1919
const field = state.private_state.get(node.property.name);
20-
2120
if (field) {
2221
return state.in_constructor ? b.member(node, b.id('v')) : b.call('$.get', node);
2322
}

packages/svelte/src/compiler/phases/3-transform/server/transform-server.js

Lines changed: 112 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -546,82 +546,112 @@ const javascript_visitors = {
546546

547547
/** @type {import('./types').Visitors} */
548548
const javascript_visitors_runes = {
549-
ClassBody(node, { state, visit, next }) {
550-
if (!state.analysis.runes) {
551-
next();
552-
}
553-
/** @type {import('estree').PropertyDefinition[]} */
554-
const deriveds = [];
555-
/** @type {import('estree').MethodDefinition | null} */
556-
let constructor = null;
557-
// Get the constructor
549+
ClassBody(node, { state, visit }) {
550+
/** @type {Map<string, import('../../3-transform/client/types.js').StateField>} */
551+
const public_derived = new Map();
552+
553+
/** @type {Map<string, import('../../3-transform/client/types.js').StateField>} */
554+
const private_derived = new Map();
555+
556+
/** @type {string[]} */
557+
const private_ids = [];
558+
558559
for (const definition of node.body) {
559-
if (definition.type === 'MethodDefinition' && definition.kind === 'constructor') {
560-
constructor = /** @type {import('estree').MethodDefinition} */ (visit(definition));
561-
}
562-
}
563-
// Move $derived() runes to the end of the body if there is a constructor
564-
if (constructor !== null) {
565-
const body = [];
566-
for (const definition of node.body) {
567-
if (
568-
definition.type === 'PropertyDefinition' &&
569-
(definition.key.type === 'Identifier' || definition.key.type === 'PrivateIdentifier')
570-
) {
571-
const is_private = definition.key.type === 'PrivateIdentifier';
572-
573-
if (definition.value?.type === 'CallExpression') {
574-
const rune = get_rune(definition.value, state.scope);
575-
576-
if (rune === '$derived') {
577-
deriveds.push(/** @type {import('estree').PropertyDefinition} */ (visit(definition)));
578-
if (is_private) {
579-
// Keep the private #name initializer if private, but remove initial value
580-
body.push({
581-
...definition,
582-
value: null
583-
});
584-
}
585-
continue;
560+
if (
561+
definition.type === 'PropertyDefinition' &&
562+
(definition.key.type === 'Identifier' || definition.key.type === 'PrivateIdentifier')
563+
) {
564+
const { type, name } = definition.key;
565+
566+
const is_private = type === 'PrivateIdentifier';
567+
if (is_private) private_ids.push(name);
568+
569+
if (definition.value?.type === 'CallExpression') {
570+
const rune = get_rune(definition.value, state.scope);
571+
if (rune === '$derived' || rune === '$derived.by') {
572+
/** @type {import('../../3-transform/client/types.js').StateField} */
573+
const field = {
574+
kind: rune === '$derived.by' ? 'derived_call' : 'derived',
575+
// @ts-expect-error this is set in the next pass
576+
id: is_private ? definition.key : null
577+
};
578+
579+
if (is_private) {
580+
private_derived.set(name, field);
581+
} else {
582+
public_derived.set(name, field);
586583
}
587584
}
588585
}
589-
if (definition.type !== 'MethodDefinition' || definition.kind !== 'constructor') {
590-
body.push(
591-
/** @type {import('estree').PropertyDefinition | import('estree').MethodDefinition | import('estree').StaticBlock} */ (
592-
visit(definition)
593-
)
594-
);
595-
}
596586
}
597-
if (deriveds.length > 0) {
598-
body.push({
599-
...constructor,
600-
value: {
601-
...constructor.value,
602-
body: b.block([
603-
...constructor.value.body.body,
604-
...deriveds.map((d) => {
605-
return b.stmt(
606-
b.assignment(
607-
'=',
608-
b.member(b.this, d.key),
609-
/** @type {import('estree').Expression} */ (d.value)
610-
)
611-
);
612-
})
613-
])
587+
}
588+
589+
// each `foo = $derived()` needs a backing `#foo` field
590+
for (const [name, field] of public_derived) {
591+
let deconflicted = name;
592+
while (private_ids.includes(deconflicted)) {
593+
deconflicted = '_' + deconflicted;
594+
}
595+
596+
private_ids.push(deconflicted);
597+
field.id = b.private_id(deconflicted);
598+
}
599+
600+
/** @type {Array<import('estree').MethodDefinition | import('estree').PropertyDefinition>} */
601+
const body = [];
602+
603+
const child_state = { ...state, private_derived };
604+
605+
// Replace parts of the class body
606+
for (const definition of node.body) {
607+
if (
608+
definition.type === 'PropertyDefinition' &&
609+
(definition.key.type === 'Identifier' || definition.key.type === 'PrivateIdentifier')
610+
) {
611+
const name = definition.key.name;
612+
613+
const is_private = definition.key.type === 'PrivateIdentifier';
614+
const field = (is_private ? private_derived : public_derived).get(name);
615+
616+
if (definition.value?.type === 'CallExpression' && field !== undefined) {
617+
const init = /** @type {import('estree').Expression} **/ (
618+
visit(definition.value.arguments[0], child_state)
619+
);
620+
const value =
621+
field.kind === 'derived_call'
622+
? b.call('$.once', init)
623+
: b.call('$.once', b.thunk(init));
624+
625+
if (is_private) {
626+
body.push(b.prop_def(field.id, value));
627+
} else {
628+
// #foo;
629+
const member = b.member(b.this, field.id);
630+
body.push(b.prop_def(field.id, value));
631+
632+
// get foo() { return this.#foo; }
633+
body.push(b.method('get', definition.key, [], [b.return(b.call(member))]));
634+
635+
if ((field.kind === 'derived' || field.kind === 'derived_call') && state.options.dev) {
636+
body.push(
637+
b.method(
638+
'set',
639+
definition.key,
640+
[b.id('_')],
641+
[b.throw_error(`Cannot update a derived property ('${name}')`)]
642+
)
643+
);
644+
}
614645
}
615-
});
616-
} else {
617-
body.push(constructor);
646+
647+
continue;
648+
}
618649
}
619-
return {
620-
...node,
621-
body
622-
};
650+
651+
body.push(/** @type {import('estree').MethodDefinition} **/ (visit(definition, child_state)));
623652
}
624-
next();
653+
654+
return { ...node, body };
625655
},
626656
PropertyDefinition(node, { state, next, visit }) {
627657
if (node.value != null && node.value.type === 'CallExpression') {
@@ -730,6 +760,16 @@ const javascript_visitors_runes = {
730760
return transform_inspect_rune(node, context);
731761
}
732762

763+
context.next();
764+
},
765+
MemberExpression(node, context) {
766+
if (node.object.type === 'ThisExpression' && node.property.type === 'PrivateIdentifier') {
767+
const field = context.state.private_derived.get(node.property.name);
768+
if (field) {
769+
return b.call(node);
770+
}
771+
}
772+
733773
context.next();
734774
}
735775
};
@@ -2089,7 +2129,8 @@ export function server_component(analysis, options) {
20892129
metadata: {
20902130
namespace: options.namespace
20912131
},
2092-
preserve_whitespace: options.preserveWhitespace
2132+
preserve_whitespace: options.preserveWhitespace,
2133+
private_derived: new Map()
20932134
};
20942135

20952136
const module = /** @type {import('estree').Program} */ (
@@ -2346,7 +2387,8 @@ export function server_module(analysis, options) {
23462387
// this is an anomaly — it can only be used in components, but it needs
23472388
// to be present for `javascript_visitors` and so is included in module
23482389
// transform state as well as component transform state
2349-
legacy_reactive_statements: new Map()
2390+
legacy_reactive_statements: new Map(),
2391+
private_derived: new Map()
23502392
};
23512393

23522394
const module = /** @type {import('estree').Program} */ (

packages/svelte/src/compiler/phases/3-transform/server/types.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type {
88
import type { SvelteNode, Namespace, ValidatedCompileOptions } from '#compiler';
99
import type { TransformState } from '../types.js';
1010
import type { ComponentAnalysis } from '../../types.js';
11+
import type { StateField } from '../client/types.js';
1112

1213
export type TemplateExpression = {
1314
type: 'expression';
@@ -35,6 +36,7 @@ export interface Anchor {
3536
export interface ServerTransformState extends TransformState {
3637
/** The $: calls, which will be ordered in the end */
3738
readonly legacy_reactive_statements: Map<LabeledStatement, Statement>;
39+
readonly private_derived: Map<string, StateField>;
3840
}
3941

4042
export interface ComponentServerTransformState extends ServerTransformState {

packages/svelte/src/internal/server/index.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
is_tag_valid_with_parent
99
} from '../../constants.js';
1010
import { DEV } from 'esm-env';
11+
import { UNINITIALIZED } from '../client/constants.js';
1112

1213
export * from '../client/validate.js';
1314

@@ -666,3 +667,17 @@ export function loop_guard(timeout) {
666667
export function inspect(args, inspect = console.log) {
667668
inspect('init', ...args);
668669
}
670+
671+
/**
672+
* @template V
673+
* @param {() => V} get_value
674+
*/
675+
export function once(get_value) {
676+
let value = /** @type {V} */ (UNINITIALIZED);
677+
return () => {
678+
if (value === UNINITIALIZED) {
679+
value = get_value();
680+
}
681+
return value;
682+
};
683+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { test } from '../../test';
2+
3+
export default test({
4+
html: `<p>a\n1</p><p>b\n2</p><p>c\n4</p>`
5+
});
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<script>
2+
class Foo {
3+
a = $state(0);
4+
c = $derived(this.b * 2);
5+
b = $derived(this.a * 2);
6+
7+
constructor(a) {
8+
this.a = a;
9+
}
10+
}
11+
12+
const foo = new Foo(1);
13+
</script>
14+
15+
<p>a {foo.a}</p>
16+
<p>b {foo.b}</p>
17+
<p>c {foo.c}</p>

0 commit comments

Comments
 (0)