Skip to content

fix(transforms): prune destructured server values - #3861

Merged
kojiwakayama merged 3 commits into
mainfrom
fix/issue-607-destructured-bindings
Aug 19, 2026
Merged

fix(transforms): prune destructured server values#3861
kojiwakayama merged 3 commits into
mainfrom
fix/issue-607-destructured-bindings

Conversation

@kojiwakayama

@kojiwakayama kojiwakayama commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Closes veryfront/veryfront-issue-inbox#607.

What changed

  • Reuse the existing pattern-binding traversal to collect binding identifier nodes as well as names.
  • Let moduleScopeDeclarations consider supported object, nested, array, and rest patterns for hook-closure pruning.
  • Keep patterns with defaults or computed keys fail-closed because evaluating them can run unrelated client code.
  • Remove a multi-binding destructuring declaration only when every binding belongs to the hook closure.
  • Preserve unrelated declarators in the same variable statement.
  • Replace the documented-limitation test with real absence assertions and add focused hazard coverage.
  • Refresh the generated template manifest required by current main after feat(integrations): run catalog tools with local credentials #3843, in a separate mechanical commit.

Root cause

moduleScopeDeclarations discarded every candidate in a VariableDeclaration as soon as one declarator used a non-Identifier binding. A destructured hook-only value therefore never reached the existing multi-name pruning consumer, so its server import survived in the browser artifact.

Safety invariant

Only plain binding positions become names and bindingIds. Patterns with computed keys or defaults remain untouched. Multi-binding patterns remain untouched unless every binding is part of the hook closure. The pass continues to under-prune when safety is uncertain.

RED-GREEN evidence

Before their implementation changes, the real-stage regressions failed for:

  • basic object destructuring
  • nested object, array, hole, and rest bindings
  • mixed variable declarators
  • effectful defaults
  • effectful computed keys
  • sibling bindings outside the hook closure

The updated focused suite passes 132 steps with 0 failures.

Verification

  • focused strip suite: 132 steps, 0 failed
  • all transform stages: 22 suites / 324 steps, 0 failed
  • deno task typecheck: pass
  • deno task lint:ci: pass
  • generated artifacts: current
  • git diff --check: pass

This is a functional transform fix only. It does not change Studio UI.

Copy link
Copy Markdown
Contributor Author

@codex Please review exact head 1e3fca6409ba909c51b371da344bdbebd8229bd3 for #607. Please focus on the safety invariant: only destructuring binding positions may be excluded from liveness; computed keys and default values must remain visible references. Also mutation-check nested/array/rest patterns and mixed declarators.

RED-GREEN evidence: all five real-stage regressions failed before implementation and now pass. Exact-tree local verification: focused suite 129 steps; all transform stages 22 suites / 321 steps; typecheck; full lint:ci; generated artifacts current.

@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 043047f5-8cd9-4a0c-a3f1-58d3554c34bd

📥 Commits

Reviewing files that changed from the base of the PR and between c4e93cd and 1e3fca6.

⛔ Files ignored due to path filters (1)
  • templates/manifest.generated.ts is excluded by !**/*.generated.*
📒 Files selected for processing (2)
  • src/transforms/pipeline/stages/browser-server-exports-strip.test.ts
  • src/transforms/pipeline/stages/browser-server-exports-strip.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The transform now analyzes destructured module-scope declarations, removes server-only bindings, and preserves pattern keys, defaults, and client-used sibling initializers. Tests cover nested, array, rest, default, computed-key, and mixed declarators.

Changes

Destructured server-value pruning

Layer / File(s) Summary
Binding analysis
src/transforms/pipeline/stages/browser-server-exports-strip.ts
Destructuring analysis collects binding identifier nodes and derives bound names from those nodes. Liveness rules exclude binding positions while retaining pattern keys and default expressions.
Module-scope pruning and validation
src/transforms/pipeline/stages/browser-server-exports-strip.ts, src/transforms/pipeline/stages/browser-server-exports-strip.test.ts
Module-scope analysis prunes fully analyzable destructured declarations. Tests cover nested, array, rest, default, computed-key, and mixed declarators.
Estimated code review effort: 3 (Moderate) ~20 minutes

Merge Risk: ⚪ Minimal · up to 1e3fc

This change correctly extends server-value pruning to supported destructured bindings while preserving references and unrelated declarators; focused and full validation pass, so no actionable merge-blocking risk remains.

Possibly related PRs

Suggested reviewers: kwakayama, mattboon

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: pruning destructured server values during transforms.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-607-destructured-bindings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

📦 Client bundle boundary

Entrypoint Modules Source size Server leaks
src/index.client.ts 326 1944 KiB ✅ 0

A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in scripts/lint/client-bundle-baseline.json to burn down.

@kojiwakayama
kojiwakayama marked this pull request as ready for review August 19, 2026 06:51

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1e3fca6409

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/transforms/pipeline/stages/browser-server-exports-strip.ts

Copy link
Copy Markdown
Contributor Author

@codex Please re-review exact head f8bdeccc4f6da638d722b6e161f3d7740c867f61. The P2 fix now fails closed for patterns with defaults or computed keys and requires every binding in a multi-binding destructuring declaration to belong to the hook closure before pruning.

Please mutation-check the three new side-effect/sibling regressions and verify that the basic, nested, array, rest, and mixed-declarator #607 cases still prune. Exact local verification: focused suite 132 steps; all transform stages 22 suites / 324 steps; typecheck; full lint:ci; generated artifacts current. Fully paginated audit: 1 thread, 0 unresolved.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

Reviewed commit: f8bdeccc4f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@kojiwakayama
kojiwakayama added this pull request to the merge queue Aug 19, 2026
Merged via the queue into main with commit a4ec5b2 Aug 19, 2026
34 checks passed
@kojiwakayama
kojiwakayama deleted the fix/issue-607-destructured-bindings branch August 19, 2026 07:26
@kwakayama kwakayama mentioned this pull request Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant