Repository navigation
fix(svelte-table, vue-table): Keep App* wrappers in sync with new cells and headers - #6633
Conversation
…ls and headers
Svelte's App* wrappers destructured their props, which read the prop getters once, and Vue's setup() read props.cell once. A keyed {#each} or v-for that reused a wrapper with a new cell or header (same id, e.g. after replacing data or columns) kept the first instance for both the snippet/slot value and the context.
Forward wrapper props through getters, read the prop reactively, and provide a live proxy that forwards to the current instance. Prototype methods run against the real instance so per-instance memos never capture the proxy.
🦋 Changeset detectedLatest commit: 9245a93 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to No actionable issue is established from the available evidence; run the adapter tests as part of normal checks before merging. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 63.64% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 6 files. (5 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
| Description check | |
The description clearly explains the bug, implementation, API impact, and regression tests. However, it omits the required "## 🎯 Changes", "## ✅ Checklist", and "## 🚀 Release Impact" sections, includi… | Add the required template sections. Complete the checklist, state whether the required tests were run or do not apply, confirm AI-assisted code review, and select the applicable release-impact option. Retain the existing technical explanati… |
✅ Passed checks (3 passed)
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Title check The title clearly identifies the fix and the affected Svelte and Vue App* wrappers. It accurately summarizes the main change.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 63.64% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 6 files. (5 skipped: 5 unsupported.)
Full details: Description check
-
Explanation
The description clearly explains the bug, implementation, API impact, and regression tests. However, it omits the required "## 🎯 Changes", "## ✅ Checklist", and "## 🚀 Release Impact" sections, including checklist confirmations and release-impact information.
Resolution
Add the required template sections. Complete the checklist, state whether the required tests were run or do not apply, confirm AI-assisted code review, and select the applicable release-impact option. Retain the existing technical explanation and test details under the Changes section.
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
-
📝 Generate docstrings 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
View your CI Pipeline Execution ↗ for commit 9245a93
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview6 package(s) bumped directly, 11 bumped as dependents. 🟩 Patch bumps
|
1 participant
Related to #6618
When
dataorcolumnsare replaced, a keyed{#each}orv-forreusesAppCell/AppHeader/AppFooterwith a new cell or header that has the same id. The wrappers kept the first instance, so the snippet/slot value anduseCellContext()/useHeaderContext()showed stale values (for example after a refetch).App*wrappers destructured their props, which reads Svelte's prop getters once. They now forwardcell,header, andchildrenthrough getters.AppCell.svelte/AppHeader.sveltederive the extended instance from the current prop.setup()readprops.cell/props.headeronce. The extended instance is now acomputed, and the slot gets its current value.createLiveProxyview that forwards reads to the current instance. Prototype methods are called on the real instance, so per-instance memos (_memo_*capturingself) never see the proxy, and forwarders are cached per key so method identity is stable.datathencolumns, and assert the snippet/slot value and both contexts update. Both tests fail onmain.Notes
useCellContext()/useHeaderContext()now return the live view, not the cell/header object itself, socontextCell === cellisfalse. The alternative is returning a getter or signal like Angular'sinjectTableCellContext(), which is cleaner but breaks the API. Solid has the same pattern but is fine with<For>(keyed by identity); it would only go stale under<Index>.Summary by CodeRabbit