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.

Sign up for GitHub

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

Jump to bottom

fix: 🐛 use unknown module type in SWC #2406

Merged
merged 4 commits into from
Apr 27, 2023
Merged

fix: 🐛 use unknown module type in SWC #2406

merged 4 commits into from
Apr 27, 2023

Conversation

JSerFeng
Copy link
Contributor

@JSerFeng JSerFeng commented Mar 21, 2023 •
edited
Loading

Summary

  • Bump SWC

  • Should parse CJS as Script instead of Module, we can let SWC automatically do that for us.

Related issue (if exists)

fixed #2339

Types of changes

  • Docs change / Dependency upgrade
  • Bug fix
  • New feature / Improvement
  • Refactoring
  • Breaking change

Checklist

  • I have added changeset via pnpm run changeset.
  • I have added tests to cover my changes.

@changeset-bot
Copy link

changeset-bot bot commented Mar 21, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: 7df5aa5

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

This PR includes changesets to release 17 packages
Name Type
@rspack/binding Patch
@rspack/postcss-loader Patch
@rspack/core Patch
webpack-test Patch
@rspack/cli Patch
@rspack/dev-middleware Patch
@rspack/dev-server Patch
@rspack/plugin-html Patch
benchmarkcase-rspack-react-refresh Patch
@rspack/dev-client Patch
@rspack/plugin-minify Patch
@rspack/plugin-node-polyfill Patch
@rspack/binding-darwin-arm64 Patch
@rspack/binding-darwin-x64 Patch
@rspack/binding-linux-x64-gnu Patch
@rspack/binding-win32-x64-msvc Patch
@rspack/fs Patch

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

@github-actions github-actions bot added the team The issue/pr is created by the member of Rspack. label Mar 21, 2023
h-a-n-a
h-a-n-a previously approved these changes Mar 22, 2023
View reviewed changes
Copy link
Contributor

@h-a-n-a h-a-n-a left a comment

Choose a reason for hiding this comment

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

@hyf0 Can you take a look at the change of tree-shaking part?

hyf0
hyf0 previously approved these changes Mar 22, 2023
View reviewed changes
@JSerFeng
Copy link
Contributor Author

Block by swc-project/swc#7118

@JSerFeng JSerFeng mentioned this pull request Mar 26, 2023
Closed
@andersk
Copy link
Contributor

andersk commented Apr 5, 2023

Block by swc-project/swc#7118

Fixed in swc ≥ 1.3.45.

@h-a-n-a
Copy link
Contributor

h-a-n-a commented Apr 6, 2023 •
edited
Loading

@JSerFeng I haven't dig into the implementation, but could you check if the PR fixes this #1793? Didn't know if allowTopLevelThis is enabled corresponding to type Unknown when matched to script, I might take manually enabling allowTopLevelThis for every module type a mistake. Related PR: #1792

@JSerFeng JSerFeng dismissed stale reviews from hyf0 and h-a-n-a via f1ab44e April 25, 2023 06:15
@JSerFeng JSerFeng force-pushed the fix/parse-module branch 2 times, most recently from 8a25954 to 2d9b08a Compare April 25, 2023 09:12
@JSerFeng
Copy link
Contributor Author

Didn't know if allowTopLevelThis is enabled corresponding to type Unknown when matched to script, I might take manually enabling allowTopLevelThis for every module type a mistake. Related PR: #1792

allow_top_level_this is placed in run_after_pass, we know if a file is strict or not at this time, and determine allow_top_level_this based on that, I think it should be OK

@JSerFeng
Copy link
Contributor Author

JSerFeng commented Apr 25, 2023 •
edited
Loading

Block by swc-project/swc#7118

Fixed in swc ≥ 1.3.45.

@andersk Sorry for the delay, I encountered some more SWC issues(7312, 7287) in this PR, so it takes this long, now is should be OK to continue

@JSerFeng JSerFeng force-pushed the fix/parse-module branch 3 times, most recently from 5b2d59a to e959710 Compare April 25, 2023 12:44
@JSerFeng
Copy link
Contributor Author

@web-infra-dev/rspack please review carefully

@ahabhgk
Copy link
Contributor

ahabhgk commented Apr 26, 2023

Some tests snapshot result and webpack.config.js are changed, could you add some explanation?

@JSerFeng
Copy link
Contributor Author

Some tests snapshot result and webpack.config.js are changed, could you add some explanation?

Stats hash changes because "use strict" disappears when moduleType is asset or script.
Some error snapshots changes because let is valid variable name in non-strict mode.

@JSerFeng
Copy link
Contributor Author

Should I extract the bump SWC part to another PR @hyf0

@JSerFeng JSerFeng added this pull request to the merge queue Apr 27, 2023
Merged via the queue into main with commit abf788a Apr 27, 2023
@JSerFeng JSerFeng deleted the fix/parse-module branch April 27, 2023 03:45
@github-actions github-actions bot mentioned this pull request May 9, 2023
Closed
siyou pushed a commit to siyou/rspack that referenced this pull request May 14, 2023
@JSerFeng @siyou
fix: 🐛 use unknown module type in SWC (web-infra-dev#2406)
a605b91 
* chore: bump-swc

* feat: parse script

* fix(visitor): handle script strict mode

* chore: update test hash
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hyf0 hyf0 hyf0 approved these changes

@ahabhgk ahabhgk ahabhgk approved these changes

@underfin underfin Awaiting requested review from underfin

@hardfist hardfist Awaiting requested review from hardfist

@h-a-n-a h-a-n-a Awaiting requested review from h-a-n-a

@IWANABETHATGUY IWANABETHATGUY Awaiting requested review from IWANABETHATGUY

Assignees
No one assigned
Labels
team The issue/pr is created by the member of Rspack.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug Report]: Parses all JS files in strict mode
6 participants
@JSerFeng @andersk @h-a-n-a @ahabhgk @IWANABETHATGUY @hyf0