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(diagnostic): improve diagnostics for swc wasm plugins when mismatch #8001

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

SyMind
Copy link
Member

@SyMind SyMind commented Sep 26, 2024

Summary

improve diagnostics when wasm plugin binary format mismatch

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

@github-actions github-actions bot added the team The issue/pr is created by the member of Rspack. label Sep 26, 2024
Copy link

netlify bot commented Sep 26, 2024

Deploy Preview for rspack ready!

Name Link
🔨 Latest commit 4ab5522
🔍 Latest deploy log https://app.netlify.com/sites/rspack/deploys/670c5c75b501f8000834870f
😎 Deploy Preview https://deploy-preview-8001--rspack.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@SyMind SyMind marked this pull request as ready for review September 26, 2024 10:52
Cargo.toml Show resolved Hide resolved
@chenjiahan chenjiahan requested a review from h-a-n-a October 12, 2024 02:56
@h-a-n-a
Copy link
Collaborator

h-a-n-a commented Oct 12, 2024

What is the context?
cc @hardfist @chenjiahan

@chenjiahan
Copy link
Member

I think the motivation for this PR is to add the test case back, @SyMind right?

@chenjiahan
Copy link
Member

We can change the PR title to make it more accurate.

@SyMind
Copy link
Member Author

SyMind commented Oct 12, 2024

@hardfist let me revert this PR. Initially, I thought swc converted panics into Result, which would allow us to modify the error handling. However, after reviewing the swc code, I found that no such change has been made.

Therefore, I believe I misunderstood the purpose of this PR. We need @hardfist to take a look at it.

@SyMind SyMind closed this Oct 12, 2024
@hardfist hardfist reopened this Oct 13, 2024
@hardfist
Copy link
Contributor

hardfist commented Oct 13, 2024

What is the context? cc @hardfist @chenjiahan

swc support throw error when wasm binary not match now, so it's safe to add this test case back without enabling catch_on_unwind
https://github.com/swc-project/swc/pull/9562/files#diff-4d01e730818bcc35a17d65dd364390283fd29e3fb1dc0e35050a67034eeb474dR111

another block in swc side https://github.com/swc-project/swc/blob/main/crates/swc/src/plugin.rs#L169, we need to remove the panic here first

@hardfist hardfist changed the title revert: revert: feat(diagnostic): improve diagnostics for swc wasm plugins fix(diagnostic): improve diagnostics for swc wasm plugins when mismatch Oct 13, 2024
@github-actions github-actions bot added the release: bug fix release: bug related release(mr only) label Oct 13, 2024
Copy link

stale bot commented Dec 13, 2024

This pull request has been automatically marked as stale because it has not had recent activity. If this pull request is still relevant, please leave any comment (for example, "bump").

@stale stale bot added the stale label Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release: bug fix release: bug related release(mr only) stale team The issue/pr is created by the member of Rspack.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants