Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 5 additions & 0 deletions .changeset/weak-sloths-laugh.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: re-run non-render-bound deriveds on the server
Original file line number Diff line number Diff line change
Expand Up @@ -68,35 +68,52 @@ function build_assignment(operator, left, right, context) {
object = object.object;
}

if (object.type !== 'Identifier' || !is_store_name(object.name)) {
if (object.type !== 'Identifier') {
return null;
}

const name = object.name.slice(1);
if (is_store_name(object.name)) {
const name = object.name.slice(1);

if (!context.state.scope.get(name)) {
return null;
if (!context.state.scope.get(name)) {
return null;
}

if (object === left) {
let value = /** @type {Expression} */ (
context.visit(build_assignment_value(operator, left, right))
);

return b.call('$.store_set', b.id(name), value);
}

return b.call(
'$.store_mutate',
b.assignment('??=', b.id('$$store_subs'), b.object([])),
b.literal(object.name),
b.id(name),
b.assignment(
operator,
/** @type {Pattern} */ (context.visit(left)),
/** @type {Expression} */ (context.visit(right))
)
);
}

if (object === left) {
const binding = context.state.scope.get(object.name);

// TODO 6.0 this won't work perfectly: once a derived is written to, it will
// no longer recompute. It might be better to disallow writing to deriveds
// on the server, to prevent this bug occurring
if (binding?.kind === 'derived' && object === left) {
let value = /** @type {Expression} */ (
context.visit(build_assignment_value(operator, left, right))
);

return b.call('$.store_set', b.id(name), value);
return b.call(binding.node, value);
}

return b.call(
'$.store_mutate',
b.assignment('??=', b.id('$$store_subs'), b.object([])),
b.literal(object.name),
b.id(name),
b.assignment(
operator,
/** @type {Pattern} */ (context.visit(left)),
/** @type {Expression} */ (context.visit(right))
)
);
return null;
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @import { Expression } from 'estree' */
/** @import { AST } from '#compiler' */
/** @import { ComponentContext } from '../types.js' */
import * as b from '#compiler/builders';
Expand All @@ -8,5 +9,5 @@ import { build_inline_component } from './shared/component.js';
* @param {ComponentContext} context
*/
export function Component(node, context) {
build_inline_component(node, b.id(node.name), context);
build_inline_component(node, /** @type {Expression} */ (context.visit(b.id(node.name))), context);
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ export function Identifier(node, context) {
return b.id('$$sanitized_props');
}

if (node.name.startsWith('$$derived_array')) {
// terrible hack, but easier than adding new stuff to `context.state` for now
return b.call(node);
}

return build_getter(node, context.state);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,52 @@ export function VariableDeclaration(node, context) {
const args = /** @type {CallExpression} */ (init).arguments;
const value = args.length > 0 ? /** @type {Expression} */ (context.visit(args[0])) : b.void0;

if (rune === '$derived.by') {
declarations.push(
b.declarator(/** @type {Pattern} */ (context.visit(declarator.id)), b.call(value))
);
if (rune === '$derived' || rune === '$derived.by') {
const is_async =
rune === '$derived' &&
context.state.analysis.async_deriveds.has(
/** @type {CallExpression} */ (declarator.init)
);

let init = is_async
? b.await(b.call('$.async_derived', b.thunk(value, true)))
: b.call('$.derived', rune === '$derived' ? b.thunk(value) : value);

if (declarator.id.type === 'Identifier') {
declarations.push(
b.declarator(/** @type {Pattern} */ (context.visit(declarator.id)), init)
);
} else {
const call = /** @type {CallExpression} */ (declarator.init);

let rhs = value;

if (rune !== '$derived' || call.arguments[0].type !== 'Identifier') {
const id = b.id(context.state.scope.generate('$$d'));

rhs = b.call(id);
declarations.push(b.declarator(id, init));
}

const { inserts, paths } = extract_paths(declarator.id, rhs);

for (const { id, value } of inserts) {
id.name = context.state.scope.generate('$$derived_array');

const expression = /** @type {Expression} */ (context.visit(b.thunk(value)));
const call = b.call('$.derived', expression);

declarations.push(b.declarator(id, call));
}

for (const path of paths) {
const expression = /** @type {Expression} */ (context.visit(path.expression));
const call = b.call('$.derived', b.thunk(expression));

declarations.push(b.declarator(path.node, call));
}
}

continue;
}

Expand All @@ -96,13 +138,6 @@ export function VariableDeclaration(node, context) {
continue;
}

if (rune === '$derived') {
declarations.push(
b.declarator(/** @type {Pattern} */ (context.visit(declarator.id)), value)
);
continue;
}

declarations.push(...create_state_declarators(declarator, context.state.scope, value));
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,10 @@ export function build_getter(node, state) {
);
}

if (binding.kind === 'derived') {
return (binding.declaration_kind === 'var' ? b.maybe_call : b.call)(binding.node);
}

return node;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
/** @import { Context as ServerContext } from '../server/types.js' */
import { extract_paths, is_expression_async } from '../../../utils/ast.js';
import * as b from '#compiler/builders';
import { get_value } from '../client/visitors/shared/declarations.js';

/**
* @template {ClientContext | ServerContext} Context
Expand Down
3 changes: 1 addition & 2 deletions packages/svelte/src/internal/client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ export {
safe_get,
tick,
untrack,
exclude_from_object,
deep_read,
deep_read_state,
active_effect
Expand All @@ -171,7 +170,7 @@ export {
} from './dom/operations.js';
export { attr, clsx } from '../shared/attributes.js';
export { snapshot } from '../shared/clone.js';
export { noop, fallback, to_array } from '../shared/utils.js';
export { noop, fallback, to_array, exclude_from_object } from '../shared/utils.js';
export {
invalid_default_snippet,
validate_dynamic_element_tag,
Expand Down
24 changes: 0 additions & 24 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -744,30 +744,6 @@ export function untrack(fn) {
}
}

/**
* @param {Record<string | symbol, unknown>} obj
* @param {Array<string | symbol>} keys
* @returns {Record<string | symbol, unknown>}
*/
export function exclude_from_object(obj, keys) {
/** @type {Record<string | symbol, unknown>} */
var result = {};

for (var key in obj) {
if (!keys.includes(key)) {
result[key] = obj[key];
}
}

for (var symbol of Object.getOwnPropertySymbols(obj)) {
if (Object.propertyIsEnumerable.call(obj, symbol) && !keys.includes(symbol)) {
result[symbol] = obj[symbol];
}
}

return result;
}

/**
* Possibly traverse an object and read all its properties so that they're all reactive in case this is `$state`.
* Does only check first level of an object for performance reasons (heuristic should be good for 99% of all cases).
Expand Down
23 changes: 18 additions & 5 deletions packages/svelte/src/internal/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { validate_store } from '../shared/validate.js';
import { is_boolean_attribute, is_raw_text_element, is_void } from '../../utils.js';
import { Renderer } from './renderer.js';
import * as e from './errors.js';
import { ssr_context } from './context.js';

// https://html.spec.whatwg.org/multipage/syntax.html#attributes-2
// https://infra.spec.whatwg.org/#noncharacter
Expand Down Expand Up @@ -457,7 +458,7 @@ export { push_element, pop_element, validate_snippet_args } from './dev.js';

export { snapshot } from '../shared/clone.js';

export { fallback, to_array } from '../shared/utils.js';
export { fallback, to_array, exclude_from_object } from '../shared/utils.js';

export {
invalid_default_snippet,
Expand All @@ -474,17 +475,29 @@ export { escape_html as escape };
* @returns {(new_value?: T) => (T | void)}
*/
export function derived(fn) {
const get_value = once(fn);
/**
* @type {T | undefined}
*/
// deriveds created during render are memoized,
// deriveds created outside (e.g. SvelteKit `page` stuff) are not
const get_value = ssr_context === null ? fn : once(fn);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this basically means that the value will run over and over on the server? FWIW I think that's fine

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactly, yeah. the alternative would basically be to implement full reactivity, which seems extravagant and unnecessary


/** @type {T | undefined} */
let updated_value;

return function (new_value) {
if (arguments.length === 0) {
return updated_value ?? get_value();
}

updated_value = new_value;
return updated_value;
};
}

/**
* @template T
* @param {()=>T} fn
*/
export function async_derived(fn) {
return Promise.resolve(fn()).then((value) => {
return () => value;
});
}
35 changes: 21 additions & 14 deletions packages/svelte/src/internal/server/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,14 @@ export class Renderer {
set_ssr_context(parent);

if (result instanceof Promise) {
result.finally(() => {
set_ssr_context(null);
});

if (child.global.mode === 'sync') {
e.await_invalid();
}

// just to avoid unhandled promise rejections -- we'll end up throwing in `collect_async` if something fails
result.catch(() => {});
child.promise = result;
Expand Down Expand Up @@ -620,24 +625,26 @@ export class Renderer {
* @returns {Renderer}
*/
static #open_render(mode, component, options) {
const renderer = new Renderer(
new SSRState(mode, options.idPrefix ? options.idPrefix + '-' : '', options.csp)
);

renderer.push(BLOCK_OPEN);

push();
if (options.context) /** @type {SSRContext} */ (ssr_context).c = options.context;
/** @type {SSRContext} */ (ssr_context).r = renderer;
var previous_context = ssr_context;

// @ts-expect-error
component(renderer, options.props ?? {});
try {
const renderer = new Renderer(
new SSRState(mode, options.idPrefix ? options.idPrefix + '-' : '', options.csp)
);

pop();
/** @type {SSRContext} */
const context = { p: null, c: options.context ?? null, r: renderer };
set_ssr_context(context);

renderer.push(BLOCK_CLOSE);
renderer.push(BLOCK_OPEN);
// @ts-expect-error
component(renderer, options.props ?? {});
renderer.push(BLOCK_CLOSE);

return renderer;
return renderer;
} finally {
set_ssr_context(previous_context);
}
}

/**
Expand Down
24 changes: 24 additions & 0 deletions packages/svelte/src/internal/shared/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,27 @@ export function to_array(value, n) {

return array;
}

/**
* @param {Record<string | symbol, unknown>} obj
* @param {Array<string | symbol>} keys
* @returns {Record<string | symbol, unknown>}
*/
export function exclude_from_object(obj, keys) {
/** @type {Record<string | symbol, unknown>} */
var result = {};

for (var key in obj) {
if (!keys.includes(key)) {
result[key] = obj[key];
}
}

for (var symbol of Object.getOwnPropertySymbols(obj)) {
if (Object.propertyIsEnumerable.call(obj, symbol) && !keys.includes(symbol)) {
result[symbol] = obj[symbol];
}
}

return result;
}
Loading
Loading