-
Notifications
You must be signed in to change notification settings - Fork 59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Maintain whitespace before tag close #822
Conversation
🦋 Changeset detectedLatest commit: a889dd8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
@@ -375,15 +375,13 @@ declare const Astro: Readonly<import('astro').AstroGlobal<%s>>`, props.Ident) | |||
p.print(`:`) | |||
p.addSourceMapping(loc.Loc{Start: eqStart + 1}) | |||
p.print(`"` + encodeDoubleQuote(a.Val) + `"`) | |||
endLoc = eqStart + 1 + len(a.Val) + 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turns out these would mess up the endLoc
because they could be out of order
internal/printer/print-to-tsx.go
Outdated
} else if unicode.IsSpace(rune(c)) { | ||
hasLeadingSpace = true | ||
endLoc++ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we find a space before the >
, we flag it
internal/printer/print-to-tsx.go
Outdated
if hasLeadingSpace { | ||
p.addSourceMapping(loc.Loc{Start: endLoc - 1}) | ||
p.print(" ") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And if we have the flag, we print an extra " "
character
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic seems fine to me, but I have one question, what happens if there's multiple spaces?
<Button >
Will they all get mapped to the first space that was created, since only one space is created?
Great question @Princesseuh! I just pushed a change to make the mapping a bit more clear for that case—that one space technically gets mapped back to the whole block of spaces in the source file. I think TypeScript should handle this just fine, but we can adjust if that's not the case. |
Changes
Testing
Two tests added
Docs
N/A, bug fix only