Skip to content

fix: preserve comment placement for server derived reads - #18641

Merged
dummdidumm merged 3 commits into
mainfrom
svelte0/svelte-18607-fix-preserve-comment-placement-for-server-derived-reads
Aug 14, 2026
Merged

fix: preserve comment placement for server derived reads#18641
dummdidumm merged 3 commits into
mainfrom
svelte0/svelte-18607-fix-preserve-comment-placement-for-server-derived-reads

Conversation

@svelte-triage-bot

@svelte-triage-bot svelte-triage-bot Bot commented Aug 14, 2026 •
edited by dummdidumm
Loading

Copy link
Copy Markdown
Contributor

Fixes #18607.

For server output, build_getter reused a derived binding's declaration identifier as the generated call callee. Esrap therefore associated leading declaration comments with an earlier reference, potentially emitting them immediately after return and triggering automatic semicolon insertion. The getter returned undefined instead of the derived value.

Fix it by avoiding the double-transform.

@pkg-svelte-dev

pkg-svelte-dev Bot commented Aug 14, 2026 •
edited
Loading

Copy link
Copy Markdown

Install the latest version of svelte from 8e5b86d:

pnpm add https://pkg.svelte.dev/svelte/c/8e5b86d052bbfd7fec356ff17f980c33f668cf22

Open in pkg.svelte.dev: https://pkg.svelte.dev/repos/svelte/pr/18641

@changeset-bot

changeset-bot Bot commented Aug 14, 2026 •
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 8e5b86d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Patch

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

@svelte-docs-bot

svelte-docs-bot Bot commented Aug 14, 2026

Copy link
Copy Markdown

Preview: https://svelte-dev-git-preview-svelte-18641-svelte.vercel.app/

@dummdidumm

dummdidumm commented Aug 14, 2026

Copy link
Copy Markdown
Member

@svelte-triage-bot compare your implementation to #18626

@svelte-triage-bot

svelte-triage-bot Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Compared with #18626, the underlying comment-placement fix is the same: use the reference location instead of binding.node’s declaration location.

The important difference is that #18626 uses the original reference node directly:

return b.call(node);

That is not safe for expressions which are revisited through a synthetic attribute. For example, with bind:value={double}, the server transform creates an attribute from the already-transformed expression and visits it again. Reusing the same identifier causes a second derived-read transform:

double()()

I verified this by applying #18626’s implementation locally: the existing writable-derived SSR, async SSR, and hydrate variants fail with TypeError: double(...) is not a function.

This PR instead:

  1. clones the reference identifier, preserving its reference source location for Esrap; and
  2. marks only that generated callee in a WeakSet, so a later synthetic-attribute visit leaves it as double() rather than transforming it again.

The focused derived-forward-ref-leading-comments|writable-derived suite passes with this implementation (18 tests).

@dummdidumm dummdidumm mentioned this pull request Aug 14, 2026
Closed
6 tasks done
@dummdidumm
dummdidumm marked this pull request as ready for review August 14, 2026 14:19
@dummdidumm
dummdidumm merged commit 221dcae into main Aug 14, 2026
14 of 15 checks passed
@dummdidumm
dummdidumm deleted the svelte0/svelte-18607-fix-preserve-comment-placement-for-server-derived-reads branch August 14, 2026 14:34
@github-actions github-actions Bot mentioned this pull request Aug 14, 2026
Merged
@GauBen GauBen mentioned this pull request Aug 18, 2026
Closed
GauBen
GauBen reviewed Aug 18, 2026
View reviewed changes
Comment thread packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/utils.js

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

@GauBen GauBen Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this code here is incorrect, see #18666

Rich-Harris pushed a commit that referenced this pull request Aug 20, 2026
@github-actions
Version Packages (#18640)
56a036f 
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## svelte@5.56.10

### Patch Changes

- fix: preserve CSS escape sequences when printing selectors
([#18667](#18667))

- fix: parse `:nth-child(2n of.foo)` where `of` is not followed by
whitespace ([#18611](#18611))

- fix: transform expressions inside labeled statements during server
compilation ([#18617](#18617))

- docs: clarify that context lookup includes the current component and
all ancestors ([#18581](#18581))

- fix: apply CSS custom properties with falsy values on components
([#18634](#18634))

- fix: correctly print `{#await ... catch x}` et al
([#18645](#18645))

- fix: ignore comments of Program node during migration script
([#18656](#18656))

- fix: reliably resolve append_style to its correct root
([#18614](#18614))

- fix: clean up removed capture event handlers from spread attributes
([#18618](#18618))

- fix: don't corrupt renderer type during SSR's legacy `bind:` retry
loop ([#18616](#18616))

- fix: treat concise arrow function bodies as implicit returns when
calculating blockers
([#18613](#18613))

- fix: give effect teardowns the value from before the first write in a
flush ([#18620](#18620))

- fix: avoid double-calling a derived reference when destructuring
`$derived` of another `$derived` during server-side rendering
([#18668](#18668))

- fix: preserve namespaces in CSS type selectors
([#18678](#18678))

- fix: increment private state fields through a non-`this` receiver
([#18622](#18622))

- chore: deduplicate client and server context helpers
([#18580](#18580))

- fix: release `last_propagated_event` after event propagation settles
so it no longer retains the last event's target subtree
([#18569](#18569))

- fix: allow custom elements to receive async values as props
([#18661](#18661))

- fix: strip comments from inline `style` values in linear time
([#18553](#18553))

- fix: prevent declaration comments from breaking server derived
references ([#18641](#18641))

- perf: make async blocker analysis scale linearly with the number of
top-level references
([#18549](#18549))

- fix: preserve short-circuiting for logical assignments to private
state fields ([#18594](#18594))

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions Bot mentioned this pull request Aug 21, 2026
Merged
dadezzz pushed a commit to dadezzz/events-cash-register that referenced this pull request Aug 25, 2026
Renovate [bot] Forgejo
fix(deps): update dependency svelte to v5.56.10 (#206)
6cb4516 
This PR contains the following updates:

| Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) |
|---|---|---|---|
| [svelte](https://svelte.dev) ([source](https://github.com/sveltejs/svelte/tree/HEAD/packages/svelte)) | [`5.56.9` → `5.56.10`](https://renovatebot.com/diffs/npm/svelte/5.56.9/5.56.10) | ![age](https://developer.mend.io/api/mc/badges/age/npm/svelte/5.56.10?slim=true) | ![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/svelte/5.56.9/5.56.10?slim=true) |

---

### Release Notes

<details>
<summary>sveltejs/svelte (svelte)</summary>

### [`v5.56.10`](https://github.com/sveltejs/svelte/blob/HEAD/packages/svelte/CHANGELOG.md#55610)

[Compare Source](https://github.com/sveltejs/svelte/compare/svelte@5.56.9...svelte@5.56.10)

##### Patch Changes

- fix: preserve CSS escape sequences when printing selectors ([#&#8203;18667](sveltejs/svelte#18667))

- fix: parse `:nth-child(2n of.foo)` where `of` is not followed by whitespace ([#&#8203;18611](sveltejs/svelte#18611))

- fix: transform expressions inside labeled statements during server compilation ([#&#8203;18617](sveltejs/svelte#18617))

- docs: clarify that context lookup includes the current component and all ancestors ([#&#8203;18581](sveltejs/svelte#18581))

- fix: apply CSS custom properties with falsy values on components ([#&#8203;18634](sveltejs/svelte#18634))

- fix: correctly print `{#await ... catch x}` et al ([#&#8203;18645](sveltejs/svelte#18645))

- fix: ignore comments of Program node during migration script ([#&#8203;18656](sveltejs/svelte#18656))

- fix: reliably resolve append\_style to its correct root ([#&#8203;18614](sveltejs/svelte#18614))

- fix: clean up removed capture event handlers from spread attributes ([#&#8203;18618](sveltejs/svelte#18618))

- fix: don't corrupt renderer type during SSR's legacy `bind:` retry loop ([#&#8203;18616](sveltejs/svelte#18616))

- fix: treat concise arrow function bodies as implicit returns when calculating blockers ([#&#8203;18613](sveltejs/svelte#18613))

- fix: give effect teardowns the value from before the first write in a flush ([#&#8203;18620](sveltejs/svelte#18620))

- fix: avoid double-calling a derived reference when destructuring `$derived` of another `$derived` during server-side rendering ([#&#8203;18668](sveltejs/svelte#18668))

- fix: preserve namespaces in CSS type selectors ([#&#8203;18678](sveltejs/svelte#18678))

- fix: increment private state fields through a non-`this` receiver ([#&#8203;18622](sveltejs/svelte#18622))

- chore: deduplicate client and server context helpers ([#&#8203;18580](sveltejs/svelte#18580))

- fix: release `last_propagated_event` after event propagation settles so it no longer retains the last event's target subtree ([#&#8203;18569](sveltejs/svelte#18569))

- fix: allow custom elements to receive async values as props ([#&#8203;18661](sveltejs/svelte#18661))

- fix: strip comments from inline `style` values in linear time ([#&#8203;18553](sveltejs/svelte#18553))

- fix: prevent declaration comments from breaking server derived references ([#&#8203;18641](sveltejs/svelte#18641))

- perf: make async blocker analysis scale linearly with the number of top-level references ([#&#8203;18549](sveltejs/svelte#18549))

- fix: preserve short-circuiting for logical assignments to private state fields ([#&#8203;18594](sveltejs/svelte#18594))

</details>

---

### Configuration

📅 **Schedule**: (UTC)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate CLI](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0NC4zMS4wIiwidXBkYXRlZEluVmVyIjoiNDQuMzEuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->
renovate Bot added a commit to cigaleapp/cigale that referenced this pull request Aug 30, 2026
@renovate
⬆️ Update dependency svelte to ^5.56.10
e54621f 
##### [v5.56.10](https://github.com/sveltejs/svelte/blob/HEAD/packages/svelte/CHANGELOG.md#55610)

##### Patch Changes

- fix: preserve CSS escape sequences when printing selectors ([#18667](sveltejs/svelte#18667))

- fix: parse `:nth-child(2n of.foo)` where `of` is not followed by whitespace ([#18611](sveltejs/svelte#18611))

- fix: transform expressions inside labeled statements during server compilation ([#18617](sveltejs/svelte#18617))

- docs: clarify that context lookup includes the current component and all ancestors ([#18581](sveltejs/svelte#18581))

- fix: apply CSS custom properties with falsy values on components ([#18634](sveltejs/svelte#18634))

- fix: correctly print `{#await ... catch x}` et al ([#18645](sveltejs/svelte#18645))

- fix: ignore comments of Program node during migration script ([#18656](sveltejs/svelte#18656))

- fix: reliably resolve append\_style to its correct root ([#18614](sveltejs/svelte#18614))

- fix: clean up removed capture event handlers from spread attributes ([#18618](sveltejs/svelte#18618))

- fix: don't corrupt renderer type during SSR's legacy `bind:` retry loop ([#18616](sveltejs/svelte#18616))

- fix: treat concise arrow function bodies as implicit returns when calculating blockers ([#18613](sveltejs/svelte#18613))

- fix: give effect teardowns the value from before the first write in a flush ([#18620](sveltejs/svelte#18620))

- fix: avoid double-calling a derived reference when destructuring `$derived` of another `$derived` during server-side rendering ([#18668](sveltejs/svelte#18668))

- fix: preserve namespaces in CSS type selectors ([#18678](sveltejs/svelte#18678))

- fix: increment private state fields through a non-`this` receiver ([#18622](sveltejs/svelte#18622))

- chore: deduplicate client and server context helpers ([#18580](sveltejs/svelte#18580))

- fix: release `last_propagated_event` after event propagation settles so it no longer retains the last event's target subtree ([#18569](sveltejs/svelte#18569))

- fix: allow custom elements to receive async values as props ([#18661](sveltejs/svelte#18661))

- fix: strip comments from inline `style` values in linear time ([#18553](sveltejs/svelte#18553))

- fix: prevent declaration comments from breaking server derived references ([#18641](sveltejs/svelte#18641))

- perf: make async blocker analysis scale linearly with the number of top-level references ([#18549](sveltejs/svelte#18549))

- fix: preserve short-circuiting for logical assignments to private state fields ([#18594](sveltejs/svelte#18594))
renovate Bot added a commit to cigaleapp/cigale that referenced this pull request Aug 30, 2026
@renovate
⬆️ Update dependency svelte to ^5.56.10 (#2101)
7caf58e 
##### [v5.56.10](https://github.com/sveltejs/svelte/blob/HEAD/packages/svelte/CHANGELOG.md#55610)

##### Patch Changes

- fix: preserve CSS escape sequences when printing selectors ([#18667](sveltejs/svelte#18667))

- fix: parse `:nth-child(2n of.foo)` where `of` is not followed by whitespace ([#18611](sveltejs/svelte#18611))

- fix: transform expressions inside labeled statements during server compilation ([#18617](sveltejs/svelte#18617))

- docs: clarify that context lookup includes the current component and all ancestors ([#18581](sveltejs/svelte#18581))

- fix: apply CSS custom properties with falsy values on components ([#18634](sveltejs/svelte#18634))

- fix: correctly print `{#await ... catch x}` et al ([#18645](sveltejs/svelte#18645))

- fix: ignore comments of Program node during migration script ([#18656](sveltejs/svelte#18656))

- fix: reliably resolve append\_style to its correct root ([#18614](sveltejs/svelte#18614))

- fix: clean up removed capture event handlers from spread attributes ([#18618](sveltejs/svelte#18618))

- fix: don't corrupt renderer type during SSR's legacy `bind:` retry loop ([#18616](sveltejs/svelte#18616))

- fix: treat concise arrow function bodies as implicit returns when calculating blockers ([#18613](sveltejs/svelte#18613))

- fix: give effect teardowns the value from before the first write in a flush ([#18620](sveltejs/svelte#18620))

- fix: avoid double-calling a derived reference when destructuring `$derived` of another `$derived` during server-side rendering ([#18668](sveltejs/svelte#18668))

- fix: preserve namespaces in CSS type selectors ([#18678](sveltejs/svelte#18678))

- fix: increment private state fields through a non-`this` receiver ([#18622](sveltejs/svelte#18622))

- chore: deduplicate client and server context helpers ([#18580](sveltejs/svelte#18580))

- fix: release `last_propagated_event` after event propagation settles so it no longer retains the last event's target subtree ([#18569](sveltejs/svelte#18569))

- fix: allow custom elements to receive async values as props ([#18661](sveltejs/svelte#18661))

- fix: strip comments from inline `style` values in linear time ([#18553](sveltejs/svelte#18553))

- fix: prevent declaration comments from breaking server derived references ([#18641](sveltejs/svelte#18641))

- perf: make async blocker analysis scale linearly with the number of top-level references ([#18549](sveltejs/svelte#18549))

- fix: preserve short-circuiting for logical assignments to private state fields ([#18594](sveltejs/svelte#18594))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dummdidumm dummdidumm dummdidumm approved these changes
+1 more reviewer
@GauBen GauBen GauBen left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Comments break server-side compiler

2 participants

@dummdidumm @GauBen