Conversation
🦋 Changeset detectedLatest commit: a33301f The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
WalkthroughThis pull request adds support for bound values with types. It includes a changeset file documenting patch version bumps for the language-server, language-tools, type-check, and marko-vscode packages. New test fixtures for return-as-type behavior are added to the language-server package. The script extractor in language-tools is modified to add spacing in the value binding output when processing bound ranges. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/language-tools/src/extractors/script/index.ts (1)
971-1002: Consider normalizing value/type spacing across all bound branches.Line 957 now inserts a separator, but member branches at Line 984 and Line 1001 still concatenate value + types directly. Unifying this logic will reduce branch-specific formatting drift.
♻️ Proposed refactor
@@ - this.#extractor - .copy(boundRange.value) - .write(" ") - .copy(boundRange.types) + this.#extractor.copy(boundRange.value); + this.#writeBoundTypes(boundRange.types); + this.#extractor .write(`\n)${SEP_COMMA_NEW_LINE}"`) @@ - this.#extractor + this.#extractor .copy(boundRange.value) .copy({ start: boundRange.value.end, end: boundRange.value.end + 1, }) .copy(boundRange.member) .copy({ start: boundRange.member.end, end: boundRange.member.end + 1, }) - .copy(boundRange.types) + this.#writeBoundTypes(boundRange.types); + this.#extractor .write(`\n)${SEP_COMMA_NEW_LINE}"`) @@ - this.#extractor - .copy(memberRange) - .copy(boundRange.types) + this.#extractor.copy(memberRange); + this.#writeBoundTypes(boundRange.types); + this.#extractor .write(`\n)${SEP_COMMA_NEW_LINE}"`)@@ + `#writeBoundTypes`(types: Range) { + if (types.start < types.end) { + this.#extractor.write(" ").copy(types); + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/language-tools/src/extractors/script/index.ts` around lines 971 - 1002, The computed-member branch adds a separator (SEP_COMMA_NEW_LINE) between the value and types before writing the trailing key, but the static-member branch (using memberRange) and the other else branch still concatenate value + types directly; update the branches that call this.#extractor.copy(memberRange or boundRange.value).copy(boundRange.types)...write(...) to insert the same separator/spacing (SEP_COMMA_NEW_LINE) and follow the identical sequence used in the boundRange.member.computed branch so value/type spacing is normalized across boundRange.member.computed, the static member branch, and any other bound branches that write the final key.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/language-tools/src/extractors/script/index.ts`:
- Around line 971-1002: The computed-member branch adds a separator
(SEP_COMMA_NEW_LINE) between the value and types before writing the trailing
key, but the static-member branch (using memberRange) and the other else branch
still concatenate value + types directly; update the branches that call
this.#extractor.copy(memberRange or
boundRange.value).copy(boundRange.types)...write(...) to insert the same
separator/spacing (SEP_COMMA_NEW_LINE) and follow the identical sequence used in
the boundRange.member.computed branch so value/type spacing is normalized across
boundRange.member.computed, the static member branch, and any other bound
branches that write the final key.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 881aecaf-b0db-474d-aacb-e55799593071
⛔ Files ignored due to path filters (6)
packages/language-server/src/__tests__/fixtures/script/return-as-type/__snapshots__/return-as-type.expected/index.htmlis excluded by!**/__snapshots__/**and included by**packages/language-server/src/__tests__/fixtures/script/return-as-type/__snapshots__/return-as-type.expected/index.mdis excluded by!**/__snapshots__/**and included by**packages/language-server/src/__tests__/fixtures/script/return-as-type/__snapshots__/return-as-type.expected/index.tsis excluded by!**/__snapshots__/**and included by**packages/language-server/src/__tests__/fixtures/script/return-as-type/__snapshots__/return-as-type.expected/tags/oneOrTwo.htmlis excluded by!**/__snapshots__/**and included by**packages/language-server/src/__tests__/fixtures/script/return-as-type/__snapshots__/return-as-type.expected/tags/oneOrTwo.mdis excluded by!**/__snapshots__/**and included by**packages/language-server/src/__tests__/fixtures/script/return-as-type/__snapshots__/return-as-type.expected/tags/oneOrTwo.tsis excluded by!**/__snapshots__/**and included by**
📒 Files selected for processing (4)
.changeset/curly-horses-behave.mdpackages/language-server/src/__tests__/fixtures/script/return-as-type/index.markopackages/language-server/src/__tests__/fixtures/script/return-as-type/tags/oneOrTwo.markopackages/language-tools/src/extractors/script/index.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
<return:=foo as Bar>was being transformed intofooas Barinstead offoo as Bar