Skip to content

feat(ts/fast-strip): Emit json errors#10144

Merged
kdy1 merged 29 commits intomainfrom
kdy1/9884
Mar 4, 2025
Merged

feat(ts/fast-strip): Emit json errors#10144
kdy1 merged 29 commits intomainfrom
kdy1/9884

Conversation

@kdy1
Copy link
Copy Markdown
Member

@kdy1 kdy1 commented Mar 4, 2025

Description:

  • Improves span for swc_fast_ts_strip.
  • Add try_with_json_handler to swc_error_reporters.
  • @swc/wasm-typescript now throws a string separated by \n.

Related issue:

@kdy1 kdy1 self-assigned this Mar 4, 2025
@kdy1 kdy1 added this to the Planned milestone Mar 4, 2025
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Mar 4, 2025

🦋 Changeset detected

Latest commit: 6d69b77

The changes in this PR will be included in the next version bump.

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

@kdy1
Copy link
Copy Markdown
Member Author

kdy1 commented Mar 4, 2025

cc @marco-ippolito Does the snapshot look good? It's json messages separated by \n, just like json rpc

@kdy1 kdy1 marked this pull request as ready for review March 4, 2025 09:03
@kdy1 kdy1 requested a review from a team as a code owner March 4, 2025 09:03
kodiakhq[bot]
kodiakhq Bot previously approved these changes Mar 4, 2025
kodiakhq[bot]
kodiakhq Bot previously approved these changes Mar 4, 2025
@marco-ippolito
Copy link
Copy Markdown
Contributor

is it possible to include the filename too?

kodiakhq[bot]
kodiakhq Bot previously approved these changes Mar 4, 2025
@kdy1 kdy1 requested a review from marco-ippolito March 4, 2025 12:07
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Mar 4, 2025

CodSpeed Performance Report

Merging #10144 will degrade performances by 5.19%

Comparing kdy1/9884 (6d69b77) with main (b145275)

Summary

⚡ 1 improvements
❌ 2 regressions
✅ 188 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
es/full/minify/libraries/moment 108.8 ms 104.1 ms +4.47%
es/minifier/libs/lodash 149.1 ms 156.1 ms -4.51%
es/minifier/libs/moment 81.8 ms 86.2 ms -5.19%

kodiakhq[bot]
kodiakhq Bot previously approved these changes Mar 4, 2025
@kdy1
Copy link
Copy Markdown
Member Author

kdy1 commented Mar 4, 2025

I'll merge/publish once @marco-ippolito approves this PR

\`----
",
}
"{"code":"InvalidSyntax","message":"await isn't allowed in non-async function","snippet":"Promise","filename":"test.ts","line":1,"column":23}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

snippet still doesnt look right.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

#10144 (comment)

It's a separate issue (a bug of parser)

Comment thread bindings/binding_typescript_wasm/__tests__/__snapshots__/transform.js.snap Outdated
Copy link
Copy Markdown
Contributor

@marco-ippolito marco-ippolito left a comment

Choose a reason for hiding this comment

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

Is it also tested in transform mode?

@kdy1
Copy link
Copy Markdown
Member Author

kdy1 commented Mar 4, 2025

Copy link
Copy Markdown
Contributor

@marco-ippolito marco-ippolito left a comment

Choose a reason for hiding this comment

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

The structure looks good to me, I think just the snippet needs to be improved

@kdy1 kdy1 enabled auto-merge (squash) March 4, 2025 12:34
@kdy1 kdy1 disabled auto-merge March 4, 2025 12:53
@kdy1 kdy1 merged commit 740bd57 into main Mar 4, 2025
@kdy1 kdy1 deleted the kdy1/9884 branch March 4, 2025 12:53
@marco-ippolito
Copy link
Copy Markdown
Contributor

marco-ippolito commented Mar 5, 2025

I think this was a breaking change since the error is emitted as a string, which is unexpected and slightly unconvinient

const inputCode = "module F { export type x = number }";
try {
	transformSync(inputCode, {
		mode: "transform",
	});
} catch (error) {
	console.log("TYPEOF OF ERROR", typeof error);
	assert.strictEqual(error.code, "UnsupportedSyntax");
}
TYPEOF OF ERROR string

@kdy1
Copy link
Copy Markdown
Member Author

kdy1 commented Mar 5, 2025

It was intentional and I wrote about it on the PR description.

@marco-ippolito
Copy link
Copy Markdown
Contributor

It was intentional and I wrote about it on the PR description.

My bad I missed it, I opened an issue to discuss #10150

@kdy1 kdy1 modified the milestones: Planned, v1.11.7 Mar 6, 2025
@swc-project swc-project locked as resolved and limited conversation to collaborators Apr 5, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

@swc/wasm-typescript improve error message output

2 participants