fix: hoisted tag vars no longer collapse to never - #582
briancarbone wants to merge 1 commit into
Conversation
Sorry, something went wrong.
🦋 Changeset detectedLatest commit: fac45fb The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 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 |
Sorry, something went wrong.
WalkthroughThe change updates Marko._.hoist so non-function inputs resolve to Exclude<T, undefined> instead of never. The style tag-variable fixture adds a top-level styles.button usage with a diagnostic marker. A changeset records patch releases for related packages, while cleanup and performance notes document program-binding source information and unconditional tag-variable hoisting. 🚥 Pre-merge checks | ✅ 5 ✅ Passed checks (5 passed)
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. ❤️ ShareComment @coderabbitai help to get the list of available commands. |
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)packages/language-server/src/__tests__/fixtures/script/style-tag-var/index.marko (1)🤖 Prompt for all review comments with AI agents1-2: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Add HTML-syntax coverage for the hoisted style variable.
This fixture only exercises concise syntax, while the PR objective requires both concise and HTML syntax. Add an equivalent non-self-closing HTML form with a styles.button marker.
🤖 Prompt for AI AgentsVerify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/language-server/src/__tests__/fixtures/script/style-tag-var/index.marko` around lines 1 - 2, Extend the style variable fixture alongside the existing styles.button concise-syntax case with an equivalent non-self-closing HTML-form element and styles.button marker. Preserve the current concise fixture and ensure both syntax forms are covered.
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. Inline comments: In `@agent-feedback/cleanup.md`: - Around line 4-9: Record both documented follow-ups in agent-feedback/README.md: add the ProgramBinding source-node/tag-kind plumbing item from agent-feedback/cleanup.md lines 4-9, and add the tag-variable hoisting performance item from agent-feedback/perf.md lines 4-9. Preserve the existing feedback details and link each README entry to its corresponding document. --- Nitpick comments: In `@packages/language-server/src/__tests__/fixtures/script/style-tag-var/index.marko`: - Around line 1-2: Extend the style variable fixture alongside the existing styles.button concise-syntax case with an equivalent non-self-closing HTML-form element and styles.button marker. Preserve the current concise fixture and ensure both syntax forms are covered.
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info ⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 00f3e69a-14e3-4c5d-a13e-24d708580f29
📥 CommitsReviewing files that changed from the base of the PR and between 832e53c and fac45fb.
⛔ Files ignored due to path filters (4)- packages/language-server/src/__tests__/fixtures/script/style-tag-var/__snapshots__/style-tag-var.expected/index.html is excluded by !**/__snapshots__/** and included by **
- packages/language-server/src/__tests__/fixtures/script/style-tag-var/__snapshots__/style-tag-var.expected/index.md is excluded by !**/__snapshots__/** and included by **
- packages/language-server/src/__tests__/fixtures/script/style-tag-var/__snapshots__/style-tag-var.expected/index.ts is excluded by !**/__snapshots__/** and included by **
- packages/language-server/src/__tests__/fixtures/script/tag-var-hoisting-from-control-flow/__snapshots__/tag-var-hoisting-from-control-flow.expected/index.md is excluded by !**/__snapshots__/** and included by **
- .changeset/hoisted-tag-var-never.md
- agent-feedback/cleanup.md
- agent-feedback/perf.md
- packages/language-server/src/__tests__/fixtures/script/style-tag-var/index.marko
- packages/language-tools/marko.internal.d.ts
Sorry, something went wrong.
|
|
||
| ## Plumb the source node (or tag kind) through `getProgramBindings` | ||
|
|
||
| `packages/language-tools/src/extractors/script/util/attach-scopes.ts` › `getProgramBindings` | 2026-07-30 | impact:low | effort:med | ||
|
|
||
| `ProgramBinding` exposes only `{ name, sources }` (plus mutation info), dropping the declaring node/tag. This makes it impossible for the script extractor's program-level hoist emission (`packages/language-tools/src/extractors/script/index.ts` › `#writeProgram`) to treat any tag kind specially — e.g. declaring a `<style/styles>` CSS-module var with its statically known selector type at program scope instead of routing it through `Marko._.hoist`. If per-tag-kind program-level typing is ever wanted, the binding needs to carry its `Node.Tag` (or at least the tag name). Re-verify: inspect the `ProgramBinding` interface in attach-scopes.ts and confirm it has no node reference. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Record actionable feedback in agent-feedback/README.md. Both additions document out-of-scope follow-ups but omit the required README record.
- agent-feedback/cleanup.md#L4-L9: add the ProgramBinding follow-up to agent-feedback/README.md.
- agent-feedback/perf.md#L4-L9: add the tag-variable hoisting performance follow-up to agent-feedback/README.md.
- agent-feedback/cleanup.md#L4-L9 (this comment)
- agent-feedback/perf.md#L4-L9
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent-feedback/cleanup.md` around lines 4 - 9, Record both documented follow-ups in agent-feedback/README.md: add the ProgramBinding source-node/tag-kind plumbing item from agent-feedback/cleanup.md lines 4-9, and add the tag-variable hoisting performance item from agent-feedback/perf.md lines 4-9. Preserve the existing feedback details and link each README entry to its corresponding document.
Source: Coding guidelines
Sorry, something went wrong.
One-line fix here: https://github.com/briancarbone/language-server/blob/fac45fb157b2ed9e523ee63c8bc012f201991189/packages/language-tools/marko.internal.d.ts#L36. The rest is agent-feedback & tests.
Fixes #581.