Conversation
🦋 Changeset detectedLatest commit: 77e0e60 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| var restore = capture(); | ||
| var value = await promise; | ||
|
|
||
| return () => { |
There was a problem hiding this comment.
is there a reason for this to be curried? from everything i saw it's always called immediately, and it certainly doesn't help minification
example:
let doubled = $derived(await wait(count * 2, 500));
// is compiled to
let doubled = (await $.save($.async_derived(async () => (await $.save(wait($.get(count) * 2, 500)))())))();
// but without the currying it could be
let doubled = (await $.save($.async_derived(async () => (await $.save(wait($.get(count) * 2, 500))))));There was a problem hiding this comment.
IIUC there's a microtask-sized gap between an async function returning and the call being evaluated:
await foo() + bar;
// ^ room here for simultaneous async work to mess with contextThe only way to plug that gap is to wrap the whole thing in a call that sets the context and then proceeds to the next expression synchronously. I am having a bit of a hard time coming up with an isolated example that proves this to be the case, but if you remove the b.call from $.save and $.track_reactivity_loss and remove the thunk wrapper from their return values, you'll see one of the tests fail
| (trimmed[0].type === 'SvelteFragment' || | ||
| trimmed[0].type === 'TitleElement' || | ||
| (trimmed[0].type === 'IfBlock' && trimmed[0].elseif)); | ||
| (trimmed[0].type === 'SvelteFragment' || trimmed[0].type === 'TitleElement'); |
There was a problem hiding this comment.
This reintroduces the many if block anchor comments that we got rid of in #15250 (copy-paste the example with 5 if branches into the async playground and see the difference in the number of comments) - why was it necessary to remove this?
There was a problem hiding this comment.
because my brain wasn't good enough to introduce async logic while keeping that stuff intact
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
elliott-with-the-longest-name-on-github
left a comment
elliott-with-the-longest-name-on-github
left a comment
There was a problem hiding this comment.
Alright, my brain is going to implode if I stare at this anymore -- it looks really good!
| * @param {Context} context | ||
| */ | ||
| export function AwaitExpression(node, context) { | ||
| let suspend = context.state.ast_type === 'instance' && context.state.function_depth === 1; |
There was a problem hiding this comment.
What's the difference between this and the new is_instance field?
There was a problem hiding this comment.
ast_type is part of AnalysisState, is_instance is part of ClientTransformState. we could align them if we wanted but i don't think it's important
packages/svelte/src/compiler/phases/3-transform/client/visitors/AwaitExpression.js
Outdated
Show resolved
Hide resolved
…s/AwaitExpression.js Co-authored-by: Elliott Johnson <sejohnson@torchcloudconsulting.com>
packages/svelte/src/compiler/phases/3-transform/client/visitors/HtmlTag.js
Show resolved
Hide resolved
5 participants
This is the PR to go with #15845. Feedback and questions should happen on the discussion rather than here.
Closes #1857, closes #3203, closes #5501
How this works
I'll likely flesh this out in more detail later, but a few quick bullet points:
Batch. This was always implied (we would flush effects in a microtask, if aflushSyncdidn't get there first, so thata++; b++would result in one flush rather than two) but now it's explicitBatchhas aprocessmethod that is responsible for callingprocess_effects. Whereas previously this would result in effects running immediately (albeit in two phases, so that e.g.$effecteffects happen after other updates), this now only happens for async effects (so that we can know if a state change resulted in async work) and block effects (so that we can know if a newly-created branch has async work, or if newly-destroyed branches can be skipped). All other effects (i.e. user effects, or those that update the DOM) are added to the batch{#if ...}etc no longer append or remove branches directly. Instead, they do that in a commit callback belonging to the batch, which runs when everything is settledprocess_effectsrun it eagerly) in a trenchcoat. This is because deriveds are lazy and would never update in time$derivedwith anawaitexpression (that isn't inside a function), or whenever you have anawaitexpression in a template effect$.template_effectnow takes a third argument, which is an array of all async deriveds{#if ...}et al) can also containawaitexpressions, in which case they are wrapped in an$.asyncblock. These simply pass the async derived to the inner logic, so that the blocks themselves don't need to be aware that they're dealing with async valuesI'm sure I'm overlooking a ton of stuff but that's as much braindump as I can give right now. Will add comments to more of the source code as and when I can.
WIP
This is not finished; there are bugs, and parts of the code are a little messy. Some stuff could possibly be extracted into a separate PR so that this one is smaller and more focused (e.g. converting boundaries to the
Boundaryclass).preserve_contextmore accurategetAbortSignalinto separate PR?Before submitting the PR, please make sure you do the following
feat:,fix:,chore:, ordocs:.packages/svelte/src, add a changeset (npx changeset).Tests and linting
pnpm testand lint the project withpnpm lint