msb-public
/
svelte
mirror of https://github.com/sveltejs/svelte
1
0
Code Issues Projects Releases Wiki Activity

fix: handle async RHS in assignment_value_stale (#17925)

Browse Source 
Fixes #17924. This also DRYs stuff a bit by making `operator` an
argument to the runtime helper function, which means we only need two
variants of it: regular and async. It also makes it so that `=`
assignments don't use the getter, because they don't need to be done
lazily.

I've added `skip_no_async` to the new test, but I'm not entirely clear
on why it was failing the TestNoAsync run to begin with.

### Before submitting the PR, please make sure you do the following

- [x] It's really useful if your PR references an issue where it is
discussed ahead of time. In many cases, features are absent for a
reason. For large changes, please create an RFC:
https://github.com/sveltejs/rfcs
- [x] Prefix your PR title with `feat:`, `fix:`, `chore:`, or `docs:`.
- [x] This message body should clearly illustrate what problems it
solves.
- [x] Ideally, include a test that fails without this PR but passes with
it.
- [x] If this PR changes code within `packages/svelte/src`, add a
changeset (`npx changeset`).

### Tests and linting

- [x] Run the tests with `pnpm test` and lint the project with `pnpm
lint`

---------

Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>
pull/17919/head
Conduitry 3 days ago committed by GitHub
parent a513da0445
commit 965f2a0ac8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 92 additions and 62 deletions
Show Stats Download Patch File Download Diff File

5
.changeset/purple-boats-hear.md
Unescape View File

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: handle async RHS in `assignment_value_stale`

44
packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js
Unescape View File

@ -5,7 +5,8 @@ import * as b from '#compiler/builders';
import {
build_assignment_value,
get_attribute_expression,
is_event_attribute
is_event_attribute,
is_expression_async
} from '../../../../utils/ast.js';
import { dev, locate_node } from '../../../../state.js';
import { build_getter, should_proxy } from '../utils.js';
@ -36,14 +37,6 @@ function is_non_coercive_operator(operator) {
return ['=', '||=', '&&=', '??='].includes(operator);
}
/** @type {Record<string, string>} */
const callees = {
'=': '$.assign',
'&&=': '$.assign_and',
'||=': '$.assign_or',
'??=': '$.assign_nullish'
};
/**
* @param {AssignmentOperator} operator
* @param {Pattern} left
@ -179,7 +172,7 @@ function build_assignment(operator, left, right, context) {
// in cases like `(object.items ??= []).push(value)`, we may need to warn
// if the value gets proxified, since the proxy _isn't_ the thing that
// will be pushed to. we do this by transforming it to something like
// `$.assign_nullish(object, 'items', () => [])`
// `$.assign(object, 'items', '??=', () => [])`
let should_transform =
dev &&
path.at(-1) !== 'ExpressionStatement' &&
@ -225,22 +218,23 @@ function build_assignment(operator, left, right, context) {
}
if (left.type === 'MemberExpression' && should_transform) {
const callee = callees[operator];
return /** @type {Expression} */ (
context.visit(
b.call(
callee,
/** @type {Expression} */ (left.object),
/** @type {Expression} */ (
left.computed
? left.property
: b.literal(/** @type {Identifier} */ (left.property).name)
),
b.arrow([], right),
b.literal(locate_node(left))
)
)
const needs_lazy_getter = operator !== '=';
const needs_async = needs_lazy_getter && is_expression_async(right);
/** @type {Expression} */
let e = b.call(
needs_async ? '$.assign_async' : '$.assign',
/** @type {Expression} */ (left.object),
/** @type {Expression} */ (
left.computed ? left.property : b.literal(/** @type {Identifier} */ (left.property).name)
),
b.literal(operator),
needs_lazy_getter ? b.arrow([], right, needs_async) : right,
b.literal(locate_node(left))
);
if (needs_async) {
e = b.await(e);
}
return /** @type {Expression} */ (context.visit(e));
}
return null;

60
packages/svelte/src/internal/client/dev/assign.js
Unescape View File

@ -21,12 +21,21 @@ function compare(a, b, property, location) {
/**
* @param {any} object
* @param {string} property
* @param {() => any} rhs_getter
* @param {string} operator
* @param {any} rhs
* @param {string} location
*/
export function assign(object, property, rhs_getter, location) {
export function assign(object, property, operator, rhs, location) {
return compare(
(object[property] = rhs_getter()),
operator === '='
? (object[property] = rhs)
: operator === '&&='
? (object[property] &&= rhs())
: operator === '||='
? (object[property] ||= rhs())
: operator === '??='
? (object[property] ??= rhs())
: null,
untrack(() => object[property]),
property,
location
@ -36,42 +45,21 @@ export function assign(object, property, rhs_getter, location) {
/**
* @param {any} object
* @param {string} property
* @param {() => any} rhs_getter
* @param {string} operator
* @param {any} rhs
* @param {string} location
*/
export function assign_and(object, property, rhs_getter, location) {
export async function assign_async(object, property, operator, rhs, location) {
return compare(
(object[property] &&= rhs_getter()),
untrack(() => object[property]),
property,
location
);
}
/**
* @param {any} object
* @param {string} property
* @param {() => any} rhs_getter
* @param {string} location
*/
export function assign_or(object, property, rhs_getter, location) {
return compare(
(object[property] ||= rhs_getter()),
untrack(() => object[property]),
property,
location
);
}
/**
* @param {any} object
* @param {string} property
* @param {() => any} rhs_getter
* @param {string} location
*/
export function assign_nullish(object, property, rhs_getter, location) {
return compare(
(object[property] ??= rhs_getter()),
operator === '='
? (object[property] = await rhs)
: operator === '&&='
? (object[property] &&= await rhs())
: operator === '||='
? (object[property] ||= await rhs())
: operator === '??='
? (object[property] ??= await rhs())
: null,
untrack(() => object[property]),
property,
location

2
packages/svelte/src/internal/client/index.js
Unescape View File

@ -1,7 +1,7 @@
export { createAttachmentKey as attachment } from '../../attachments/index.js';
export { FILENAME, HMR, NAMESPACE_SVG } from '../../constants.js';
export { push, pop, add_svelte_meta } from './context.js';
export { assign, assign_and, assign_or, assign_nullish } from './dev/assign.js';
export { assign, assign_async } from './dev/assign.js';
export { cleanup_styles } from './dev/css.js';
export { add_locations } from './dev/elements.js';
export { hmr } from './dev/hmr.js';

25
packages/svelte/tests/runtime-runes/samples/assignment-value-stale-lazy-rhs-async/_config.js
Unescape View File

@ -0,0 +1,25 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
compileOptions: {
dev: true
},
async test({ assert, target }) {
const button = /** @type {HTMLElement} */ (target.querySelector('button'));
await tick();
assert.htmlEqual(target.innerHTML, `<button>go</button><p>count1: 0, count2: 0</p>`);
button.click();
await tick();
assert.htmlEqual(target.innerHTML, `<button>go</button><p>count1: 1, count2: 1</p>`);
// additional tick necessary in legacy mode because it's using Promise.resolve() which finishes before the await in the component,
// causing the cache to not be set yet, which would result in count2 becoming 2
await tick();
button.click();
await tick();
assert.htmlEqual(target.innerHTML, `<button>go</button><p>count1: 2, count2: 1</p>`);
}
});

18
packages/svelte/tests/runtime-runes/samples/assignment-value-stale-lazy-rhs-async/main.svelte
Unescape View File

@ -0,0 +1,18 @@
<script>
let count1 = $state(0);
let count2 = $state(0);
let cache = $state({});
async function go() {
count1++;
const value = cache.value ??= await get_value();
}
function get_value() {
count2++;
return 42;
}
</script>
<button onclick={go}>go</button>
<p>count1: {count1}, count2: {count2}</p>
Write Preview
Loading…
Cancel
Save