fix: organizeImports adding newlines in HTML-ish templating languages when :BLANK_LINE: was involved#9592
Draft
fix: organizeImports adding newlines in HTML-ish templating languages when :BLANK_LINE: was involved#9592
:BLANK_LINE: was involved#9592Conversation
🦋 Changeset detectedLatest commit: 53db612 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 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 |
ematipico
reviewed
Mar 23, 2026
… when `:BLANK_LINE:` was involved
ematipico
requested changes
Mar 23, 2026
Member
ematipico
left a comment
There was a problem hiding this comment.
The fix isn't entirely correct. There's an evident regression in the snapshots
Comment on lines
+42
to
+45
| 1 │ - import·{·getLocale·}·from·"astro:i18n"; | ||
| 2 │ - import·{·Code·}·from·"astro:components"; | ||
| 1 │ + import·{·Code·}·from·"astro:components"; | ||
| 2 │ + import·{·getLocale·}·from·"astro:i18n"; |
Member
There was a problem hiding this comment.
Do you see the bug? The first import now is at line 1, which is wrong. It should be at line 2, like before
Contributor
Author
There was a problem hiding this comment.
Yes, I did. I'm still mulling over whether or not this is the right fix. I'll mark it ready to review when its ready to review.
Comment on lines
+83
to
+95
| /// Returns the byte range of the actual frontmatter code inside the `---` fences. | ||
| /// | ||
| /// Astro frontmatter commonly includes a newline immediately after the opening | ||
| /// fence and before the closing fence. Those wrapper-adjacent bytes are part of | ||
| /// the host document layout, but they should not be treated as part of the | ||
| /// embedded JavaScript/TypeScript snippet itself. | ||
| /// | ||
| /// Keeping them in the embedded slice makes snippet-based assists like | ||
| /// `organizeImports` observe a different input than standalone JS/TS, which can | ||
| /// lead to incorrect blank-line edits at the snippet boundary. By trimming only | ||
| /// the fence-adjacent whitespace, we keep parser offsets and fix application | ||
| /// aligned to the meaningful frontmatter content while still preserving the host | ||
| /// document's surrounding layout. |
Member
There was a problem hiding this comment.
Here's the problem of the agents: they write the comments as a "story" of things they're fixing, with the given context:
- it talks about import sorting, when it's not necessarily the cause of the bug
- it talks about Astro when in reality we are fixing a more general problem
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.
Summary
generated by gpt 5.4
This one was a bit weird because the bug happened across both with and without full html support, but only on astro. vue and svelte was bugged only with full html support enabled.
Essentially, this was an interaction between how we extract embedded snippets vs how the rule deals with the
:BLANK_LINE:group.fixes #9097
Test Plan
Docs