Per
https://github.com/sveltejs/svelte/pull/17862#issuecomment-4049752548,
this freezes the value of a derived if it was created inside a parent
effect that is now destroyed. This prevents the sort of bug where a
derived reads `foo.bar` even though `foo` is now `undefined`.
If the derived is dirty, a warning will be printed.
This PR also gets rid of some weirdness around `derived.parent` — it can
only ever be an `Effect | null`, and there's no need for
`get_derived_parent_effect`.
Blocked on https://github.com/sveltejs/kit/pull/15533 and a follow-up
that switches remote functions to use `$effect.root` (since this
effectively undoes #17171), hence draft.
### 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: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
## Description
**While investigating the compiler analysis phase for errors or missing
lines, I located an explicit `TODO fix the message here` comment in the
source code
(`packages/svelte/src/compiler/phases/2-analyze/visitors/shared/utils.js`).**
### Triggering Code
```svelte
<script module>
import { foo } from './somewhere.js';
</script>
<script>
let foo = 'conflict'; // Triggers the duplicate module import error
</script>
```
## The Bug
When a component author creates a local `let` declaration in the
instance `<script>` block that shadows a variable imported in the
`<script module>` block, the compiler emits a
`declaration_duplicate_module_import` error.
However, the error message text was misleading. It stated:
```diff
- Cannot declare a variable with the same name as an import inside `<script module>`
+ Cannot declare a variable with the same name as an import from `<script module>`
```
This incorrectly implies that the duplicate declaration itself is
happening *inside* the module script block, rather than colliding with
an import *from* that block.
## Changes Made
| Action | Target (File / Function) | Description |
| :--- | :--- | :--- |
| **Template Fix** | `packages/svelte/messages/compile-errors/script.md`
| Changed "inside" to "from" to accurately reflect the nature of the
shadow collision. |
| **Regenerated Errors** | `packages/svelte/src/compiler/errors.js` |
Ran `pnpm generate` to apply the updated markdown template into the
dictionary list. |
| **Removed TODO** | `ensure_no_module_import_conflict` function |
Removed the original `// TODO fix the message here` notation from the
validator. |
| **Test Adjustments** | `illegal-variable-declaration/errors.json` |
Updated the validator test suite snapshots to assert against the correct
error message format. |
## Validation Results
* The error correctly throws when a user replicates this component
structure.
* Vitest suite `tests/validator/test.ts` successfully asserts the new,
clearer message.
---
### 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`
# THANK YOU
With this, we can add invariants to the codebase so we can identify
problems like 'this batch already has roots scheduled', which indicate a
bug somewhere, without a) needing tests for scenarios that are
inherently hard to anticipate, or b) cluttering people's prod bundles
## Summary
Fixes#17721
In dev mode, detect when a keyed each block has a key function that
returns different values when called multiple times for the same item
(non-idempotent). This catches the common mistake of using array
literals like `[thing.group, thing.id]` as keys, which creates a new
array object each time and will never match by reference.
- Adds new `each_key_volatile` error with helpful message explaining the
issue
- Checks key idempotency in the each block loop during dev mode
- Provides a clear error instead of the cryptic "Cannot read properties
of undefined" that occurred previously
---------
Co-authored-by: 7nik <kifiranet@gmail.com>
Since async-await was introduced into the code base a lot has changed. This lifts the restriction.
Closes#17131 (though I still wonder why Skeleton does that)
* fix: take async into account for bindings/transitions/animations/attachments
- block on async work
- error at compile time on await expressions. Right now it gives confusing errors later at compile time or at runtime
Fixes#17194
* this was weird
* chore: run boundary async effects in the context of the current batch
* WIP
* reinstate kludge
* fix test
* WIP
* WIP
* WIP
* remove kludge
* restore batch_values after commit
* make private
* tidy up
* fix tests
* update test
* reset #dirty_effects and #maybe_dirty_effects
* add test
* WIP
* add test, fix block resolution
* bring async-effect-after-await test from defer-effects-in-pending-boundary branch
* avoid reawakening committed batches
* changeset
* cheat
* better API
* regenerate
* slightly better approach
* lint
* revert this whatever it is
* add test
* Update feature description for fork API
* error if missing experimental flag
* rename inspect effects to eager effects, run them in prod
* regenerate
* Apply suggestions from code review
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
* tidy up
* add some minimal prose. probably don't need to go super deep here as it's not really meant for non-framework authors
* bit more detail
* add a fork_timing error, regenerate
* unused
* add note
* add fork_discarded error
* require users to discard forks
* add docs
* regenerate
* tweak docs
* fix leak
* fix
* preload on focusin as well
* missed a spot
* reduce nesting
---------
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
* feat: First pass at payload
* first crack
* snapshots
* checkpoint
* fix: cloning
* add test option
* big dumb
* today's hard work; few tests left to fix
* improve
* tests passing no wayyyyy yo
* lots of progress, couple of failing tests around selects
* meh
* solve async tree stuff
* fix select/option stuff
* whoop, tests
* simplify
* feat: hoisting
* fix: `$effect.pending` sends updates to incorrect boundary
* changeset
* stuff from upstream
* feat: first hydrationgaa
* remove docs
* snapshots
* silly fix
* checkpoint
* meh
* ALKASJDFALSKDFJ the test passes
* chore: Update a bunch of tests for hydration markers
* chore: remove snippet and is_async
* naming
* better errors for sync-in-async
* test improvements
* idk man
* merge local branches (#16757)
* use fragment as async hoist boundary
* remove async_hoist_boundary
* only dewaterfall when necessary
* unused
* simplify/fix
* de-waterfall awaits in separate elements
* update snapshots
* remove unnecessary wrapper
* fix
* fix
* remove suspends_without_fallback
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
* Update payload.js
Co-authored-by: Rich Harris <rich.harris@vercel.com>
* checkpoint
* got the extra children to go away
* just gonna go ahead and merge this as the review comments take up too much space
* chore: remove hoisted_promises (#16766)
* chore: remove hoisted_promises
* WIP optimise promises
* WIP
* fix <slot> with await in prop
* tweak
* fix type error
* Update packages/svelte/src/compiler/phases/3-transform/server/visitors/SvelteHead.js
* chore: fix hydration treeshaking (#16767)
* chore: fix hydration treeshaking
* fix
* remove await_outside_boundary error (#16762)
* chore: remove unused analysis.boundary (#16763)
* chore: simplify slots (#16765)
* chore: simplify slots
* unused
* Apply suggestions from code review
* chore: remove metadata.pending (#16764)
* Update packages/svelte/src/compiler/phases/3-transform/server/visitors/SnippetBlock.js
* put this back where it was, keep the diff small
* Update packages/svelte/src/compiler/phases/types.d.ts
Co-authored-by: Rich Harris <rich.harris@vercel.com>
* chore: remove analysis.state.title (#16771)
* chore: remove analysis.state.title
* unused
* chore: remove is_async (#16769)
* chore: remove is_async
* unused
* Apply suggestions from code review
Co-authored-by: Rich Harris <rich.harris@vercel.com>
* cleanup
* lint
* clean up payload a bit
* compiler work
* run ssr on sync and async
* prettier
* inline capture
* Update packages/svelte/src/compiler/phases/3-transform/server/visitors/EachBlock.js
* chore: simplify build_template (#16780)
* small tweak to aid greppability
* chore: fix SSR context (#16781)
* at least passing
* cleanup
* fix
* remove push/pop from exports, not needed with payload
* I think this is better but tbh not sure
* async SSR
* qualification
* errors:
* I have lost the plot
* finally
* ugh
* tweak error codes to better align with existing conventions, such as they are
* tweak messages
* remove unused args
* DRY out a bit
* unused
* unused
* unused
* simplify - we can enforce readonly at a type level
* unused
* simplify
* avoid magical accessors
* simplify algorithm
* unused
* unused
* reduce indirection
* TreeState -> SSRState
* mark deprecated methods
* grab this.local from parent directly
* rename render -> fn per conventions (fn indicates 'arbitrary code')
* reduce indirection
* Revert "reduce indirection"
This reverts commit 3ec461baad.
* tweak
* okay works this time
* no way chat, it works
* fix context stuff
* tweak
* make it chainable
* lint
* clean up
* lint
* Update packages/svelte/src/internal/server/types.d.ts
Co-authored-by: Rich Harris <rich.harris@vercel.com>
* sunset html for async
* types
* we use 'deprecated' in other messages
* oops
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
Implemented by reusing the `async_body` function inside `Fragment.js`. Also removes the ability to reference a `{@const ...}` of an implicit child inside a boundary pending/failed snippet:
- existing duplication of consts can have unintended side effects, e.g. async consts would unexpectedly called multiple times
- what if a const is the reason for the failure of a boundary, but is then referenced in the failed snippet?
- what if an async const is referenced in a pending snippet? deadlock
- inconsistent with how it behaves for components where this already does not work
Implemented via the experimental flag so the behavior change only applies there as this is a breaking change strictly speaking. Also added a compiler error for this.
closes#16462
* Add missing hyphen in "server-side rendering"
* Fix incorrect use of "as such"
* regenerate types
---------
Co-authored-by: Chew Tee Ming <chew.tee.ming@nindatech.com>
* use sh instead of bash in source blocks
no bash-specific functionality is used
* regenerate
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
* fix: throw on duplicate class field declarations
* doh
* handle assignment in constructor
* fix
* apply suggestion from review
* fix
* fix failing test
Fixes#16134
* Add a warning when the misuse of `reset` in an `error:boundary` causes an error to be thrown when flushing the boundary content
* Prevent uncaught errors to make test fails when they are expected and are fired during template effects flush
* reset should just be a noop after the first call
* correctly handle errors inside boundary during reset
* handle errors in the correct boundary
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
* tidy
* tidy
* yes it can, apparently
* tidy up
* unused
* complete merge
* WIP
* simplify
* debugging help
* WIP
* unused
* partial merge
* WIP
* fix
* add test
* rename
* fix
* unused
* oops
* chore: merge main into async branch (#16197)
* chore: merge main into async branch
* adjust test
* fix: make effects depend on state created inside them (#16198)
* make effects depend on state created inside them
* fix, add github action
* disable test in async mode
* make batch.#deferred private
* fix settled when awaits occur inside pending boundary
* tweak
* change behaviour of `tick()` to be requestAnimationFrame-based
* get rid of a bunch of Promise.resolve chains
* more
* more
* fix test
* disallow `flushSync()` inside effects
* regenerate
* handle errors in block expressions
* make validate_each_keys async-aware
* for unowned deriveds, throw errors lazily
* rename ASYNC_ERROR -> ERROR_VALUE, and avoid conflicts with other flags now that it's used with deriveds as well as sources
* invoke boundary directly
* local effect pending
* update test
* fix
* fix
* fix weird bug in tests
* delete old changeset that somehow got left over here
* Update .changeset/eleven-weeks-dance.md
* update error details
* unused
* simplify
* tweak
* tweak
* tweak
* tweak
* tidy up
* handle errors in async block expressions
* tweak
* groundwork for async attribute_effect
* dry out
* fix async directives
* tidy up
* initialize option values before initing select values
* simplify init_select
* simplify
* tweak
* tidy up
* tweak
* on second thoughts just simplify it here
* tidy
* handle awaits in `<slot>`
* unused
* tidy up
* tidy up
* dry out
* dry out
* Revert "dry out"
This reverts commit 25855163bf.
* dry out
* dry out
* use let for block-scoped stuff
* dry out
* dry out
* tidy up
* only wrap awaits in `$.save` when necessary
* oops
* remove TODO comment (just checked)
* oops, leftover
* simplify
* unused
* remove logging
* tweak
* unused
* unused
* remove logging
* partial fix
* fix
* remove unused EFFECT_HAS_DERIVED
* Update packages/svelte/src/reactivity/create-subscriber.js
Co-authored-by: Elliott Johnson <sejohnson@torchcloudconsulting.com>
* Update packages/svelte/src/index-client.js
Co-authored-by: Elliott Johnson <sejohnson@torchcloudconsulting.com>
* Update packages/svelte/src/internal/client/runtime.js
Co-authored-by: Elliott Johnson <sejohnson@torchcloudconsulting.com>
* unused
* Update packages/svelte/src/internal/client/reactivity/sources.js
Co-authored-by: Elliott Johnson <sejohnson@torchcloudconsulting.com>
* Update packages/svelte/src/internal/client/reactivity/deriveds.js
Co-authored-by: Elliott Johnson <sejohnson@torchcloudconsulting.com>
* Update packages/svelte/src/internal/client/reactivity/deriveds.js
Co-authored-by: Elliott Johnson <sejohnson@torchcloudconsulting.com>
* prettier
* unused
* fix flags
* tweak
* tweak
* unused
* fix
* no idea what a 'boundary micro task' is or why it was deemed necessary but evidently it isn't
* remove queue_boundary_micro_task
* oops
* note
* tidy up
* remove TODO
* make method private
* simplify
* flesh out await_reactivity_loss warning
* tweak
* update test
* fix
* null out from_async_derived in more places
* tidy up test
* failing test
* unused
* fix test
* fix
* simplify. no idea what the async_mode_flag stuff is about, but it appears unnecessary
* add async_derived_orphan error
* regenerate
* flesh out await_outside_boundary message
* add some JSDoc
* only update `$effect.pending()` if someone is listening, since it causes a double flush and makes debugging harder
* tweak logic to make it clearer why and when we commit a batch
* add a couple of comments
* false -> 0
* add comment
* unused
* silence warning
* add effect_pending_outside_reaction error
* Update packages/svelte/src/compiler/types/index.d.ts
Co-authored-by: Elliott Johnson <sejohnson@torchcloudconsulting.com>
* suspend batch, not boundary
* rename from_async_derived -> current_async_derived
* tweak
* remove TODO - this method is only called when pending snippet exists
* use error boundary for test - vitest does some weird error swallowing afaict
* flush less often
* restore -> activate
* remove TODO
* move batch-related code into batch.js
* make flush_queued_root_effects a method of batch
* make process_effects a method of batch
* make stuff private
* unused
* regenerate
* update test
* more JSDoc
* add more JSDoc
* branch and block effects do not also need to be render effects
* tidy up
* simplify
* unused
* move code where it belongs
* remove, for now
* fix
* only apply error adjustments when error escapes boundaries
* remove EFFECT_IS_UPDATING
* is_dirty is a better name than check_dirtiness
* duplicates are rare and harmless
* apparently we no longer need the merging logic? we can simplify and fix stuff by removing it
* tidy
* don't commit stale batches
* add skipped failing test
* partial merge
* WIP
* WIP
* WIP
* tweak
* tidy up
* dont update derived status when time-travelling
* tidy up
* tidy up
* tag async deriveds
* tweak
* bail out of secondary flushes
* re-run blocks on subsequent flushes
* add test
* fix
* add tests, one failing
* fix
* flesh out await_waterfall message
* tidy up
* dry out
* unused
* tweak
* tidy up
* TODO
* tweak
* tidy up
* remove TODO
* unused export
* add optimisation back
* revert unneeded changes
* revert
* update some tests
* more
* more
* move some code
* rename
* WIP
* unset context synchronously
* remove unused argument
* Apply suggestions from code review
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
* add comment
* add comment
* use queue_micro_task in createSubscriber
* Update packages/svelte/src/compiler/phases/3-transform/client/visitors/AwaitExpression.js
Co-authored-by: Elliott Johnson <sejohnson@torchcloudconsulting.com>
* prettier
---------
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>
Co-authored-by: Elliott Johnson <sejohnson@torchcloudconsulting.com>
* fix: warn when using rest or identifier in custom elements without props option
* chore: update message
* tweak message
* update tests
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
* feat: State declarations in class constructors
* feat: Analysis phase
* misc
* feat: client
* improvements
* feat: It is now at least backwards compatible. though the new stuff may still be wrong
* feat: It works I think?
* final cleanup??
* tests
* test for better types
* changeset
* rename functions (the function doesn't test call-expression-ness)
* small readability tweak
* failing test
* fix
* disallow computed state fields
* tweak message to better accommodate the case in which state is declared after a regular property
* failing test
* wildly confusing to have so many things called 'class analysis' - rename the transform-phrase ones to transformers
* missed a spot
* and another
* store analysis for use during transformation
* move code to where it is used
* do the analysis upfront, it's way simpler
* skip failing test for now
* simplify
* get rid of the class
* on second thoughts
* reduce indirection
* make analysis available at transform time
* WIP
* WIP
* WIP
* fix
* remove unused stuff
* revert snapshot tests
* unused
* note to self
* fix
* unused
* unused
* remove some unused stuff
* unused
* lint, tidy up
* reuse helper
* tweak
* simplify/DRY
* unused
* tweak
* unused
* more
* tweak
* tweak
* fix proxying logic
* tweak
* tweak
* adjust message to accommodate more cases
* unskip and fix test
* fix
* move
* revert unneeded drive-by change
* fix
* fix
* update docs
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
* fix: warn on bidirectional control characters
* check evaluated values as well, fix minor issue
* fix failing tests
* lint
* fix
* shrink warning code
* use validator test suite rather than snapshot (which should be used sparingly as it creates more git noise)
* show ranges during parsing, and warn on all occurrences rather than just the first
* fix lint
* move check into Text visitor so it happens in expected order
* unused
* add svelte-ignore test
* ignore control characters following a svelte-ignore comment
* tweak message
* no need to test evaluations, since we are already testing the literals that they are composed of
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
* init
* fix
* make `Payload` a class
* doh
* lint
* tweak changeset
* fix
* only export things that should be available on $
* tweak message
* fix
---------
Co-authored-by: Rich Harris <rich.harris@vercel.com>
Rich Harris
Rohit Nair P
Elliott Johnson
Bladesheng
Pooh
Elliott Johnson
ComputerGuy
Alejandro Torres
Simon H
Jakub Kolesár
Anthony
fkobi
Ray Thurn Void
Paolo Ricciuti
Gautier Ben Aïm
7nik