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(babel): make sure not read browserslist #10396

Merged
merged 4 commits into from
Feb 6, 2023
Merged

Conversation

fz6m
Copy link
Contributor

@fz6m fz6m commented Feb 3, 2023

fix #10356

  1. 添加配置,确保项目中的明面代码不读 browserslist ,因为有 targets 了,再去读 browserslist 会造成意外、非预期的后果;但这不能防止暗面上有 babel 插件去读 browserslist (因为不置为 false 还是会自动默认读)。

  2. 通过观察 babel 源码发现,我们不能 hack 到所有自动读 browserslist 的入口,因为太多了。所以在暗面上,即使有 babel 插件读 browserslist ,我们要防止他不能报错。当使用了 extends browserlist 时,会导致无法加载报错,故解法是在预打包阶段 patch 成 eval 防止被打包成空模块。

该问题仅发生在自动寻路时可以发现 browserslist 配置的 case (比如 .browserslistrcpackage.json#browserslist,较少见),可以继续观察是否有新反馈。

@vercel
Copy link

vercel bot commented Feb 3, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated
umi ⬜️ Ignored (Inspect) Feb 3, 2023 at 8:18AM (UTC)

@codecov
Copy link

codecov bot commented Feb 3, 2023

Codecov Report

Base: 29.29% // Head: 29.29% // No change to project coverage 👍

Coverage data is based on head (cf34332) compared to base (00b211d).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #10396   +/-   ##
=======================================
  Coverage   29.29%   29.29%           
=======================================
  Files         445      445           
  Lines       13006    13006           
  Branches     3178     3178           
=======================================
  Hits         3810     3810           
  Misses       8592     8592           
  Partials      604      604           
Impacted Files Coverage Δ
...ages/bundler-webpack/src/config/javaScriptRules.ts 74.28% <ø> (ø)
packages/lint/src/config/eslint/index.ts 0.00% <ø> (ø)
packages/lint/src/config/eslint/legacy.ts 0.00% <ø> (ø)
packages/preset-umi/src/features/legacy/legacy.ts 0.00% <ø> (ø)
...kages/preset-umi/src/features/polyfill/polyfill.ts 0.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@fz6m fz6m mentioned this pull request Feb 3, 2023
@fz6m
Copy link
Contributor Author

fz6m commented Feb 3, 2023

umi 4 没有读 browserslist 的设计,我对修复该问题持中立态度,用户不应该配置 browserslist 。

See #10389 (comment)

@vagusX
Copy link

vagusX commented Feb 4, 2023

umi 4 没有读 browserslist 的设计,我对修复该问题持中立态度,用户不应该配置 browserslist 。

See #10389 (comment)

用户为啥不应该配置 browserslist 呢 😂

nodePartFile,
originContent.replace(
/require\(require\.resolve/g,
'eval("require")(require.resolve',
Copy link
Member

Choose a reason for hiding this comment

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

eval("require") 的作用是啥?

Copy link
Member

@sorrycc sorrycc Feb 6, 2023

Choose a reason for hiding this comment

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

为了绕过预打包过程中对 require.resolve 的处理?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

如果就这样预打包的话,实际上 ncc 是打包了个 empty module ,导致出现 issue 反馈的问题。

因为他是动态加载的,所以需要 eval 的 hack 手法,让 ncc 不打包这句话,在 nextjs 里对 sass-loader 动态加载 sass 也是这么处理的。

@sorrycc
Copy link
Member

sorrycc commented Feb 6, 2023

umi 4 没有读 browserslist 的设计,我对修复该问题持中立态度,用户不应该配置 browserslist 。
See #10389 (comment)

用户为啥不应该配置 browserslist 呢 😂

因为有 targets 配置了,框架层希望做配置收敛。

@sorrycc sorrycc merged commit f4ef0ca into umijs:master Feb 6, 2023
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.

[Bug] browserslist 包中 require.resolve 报错
3 participants