-
-
Notifications
You must be signed in to change notification settings - Fork 54
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
Properly account for new TSX prefix #733
Conversation
🦋 Changeset detectedLatest commit: 873a126 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 |
@@ -0,0 +1,6 @@ | |||
--- |
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.
This needs to be in a real file now because otherwise it cannot seems to resolve astro/jsx-runtime
, not sure why. Might be something with how fake files works in tests.
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.
As much as I am able to wrap my head around this it LGTM!
Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
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.
LGTM!
Changes
Fixing
astro check
's checking behaviour so it checks.js(x)
files properly revealed a problem: We forceastroHTML
's types in every file types. This is not a problem in the editor really, because in Astro projects you kinda don't rely on the types of JSX components inside Astro files, but inastro check
, this is obviously wrong.In withastro/compiler#917, I made it so our TSX is now prefixed with a JSX pragma, that way we don't need to force a JSX setting on every file, our files just set the proper runtime types to use automatically. Unfortunately, this is quite rigid and requires exporting the JSX types from somewhere named
package/jsx-runtime
, so I did that in Astro's JSX runtime in withastro/astro#9501This is all great and awesome, however a lot of our mapping around auto imports and code actions depended on the frontmatter being the first thing in the generated TSX and JSX pragmas needs to be on the first lines of TSX files, so in withastro/compiler#931, I made it so the compiler now returns the ranges of the TSX inside the virtual file, that way we can map directly to there. This was a fair amount of work.
The diff is larger than it might seems due to many dependencies updates needed for kinda unrelated things, Volar's current essentially pre-1.0 state suggest updating everything to be at the same exact version when you update one thing. The Astro version used for tests was also updated to account for the jsx-runtime export
Fix #727 (I'll remove the
astro check
hotfix workaround in a separate PR)Fix #715 by accident too
Testing
Tests should pass! Updated the unit tests for the utils to match the new ranges they return (since they now return ranges in the TSX vs ranges in the Astro file, they might look inaccurate, but as far as I can tell, everything is okay!)
Docs
N/A