Skip to content
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

fix(sveltekit): Correctly parse angle bracket type assertions for auto instrumentation #15578

Merged
merged 6 commits into from
Mar 11, 2025

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Mar 4, 2025

This PR

  • removes magicast as a dependency of the Sveltekit SDK
  • replaces it with direct usage of recast and @babel/parser. Both dependencies were already transitive dependencies beforehand but with this, we just hoist it up a level.
  • configures recast to use the TypeScript parser, just like it configures it. We can't directly use the recast typscript parser because its export is not compatible with ESM
  • Makes adjustments to our code (mostly type casts) due to slightly changed types.
  • Given that we have various tests that cover our AST walking logic I think this is a low-risk change overall.

closes #9318

@@ -101,22 +103,80 @@ export async function canWrapLoad(id: string, debug: boolean): Promise<boolean>

const code = (await fs.promises.readFile(id, 'utf8')).toString();

const mod = parseModule(code);
// Taken from recast's typescript parser config, minus the JSX plugin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we're vendoring this in, we need to include the license. Would make sense to extract it to another file then.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not a 1:1 copy but I agree, close enough to include the license and some more information. Thanks for the suggestion! c07db78

@Lms24 Lms24 self-assigned this Mar 4, 2025
@Lms24 Lms24 requested a review from AbhiPrasad March 4, 2025 17:17
@Lms24 Lms24 force-pushed the lms/fix-sveltekit-type-assertion branch from ae5716b to ed21615 Compare March 10, 2025 13:19
Lms24 added 6 commits March 11, 2025 10:30
…o instrumentation

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@Lms24 Lms24 force-pushed the lms/fix-sveltekit-type-assertion branch from ed21615 to 62f3b09 Compare March 11, 2025 09:30
@Lms24 Lms24 merged commit d0b2f7d into develop Mar 11, 2025
150 checks passed
@Lms24 Lms24 deleted the lms/fix-sveltekit-type-assertion branch March 11, 2025 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SvelteKit auto-instrumentation causes faulty JSX error
2 participants