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

node_modules下的tsx也运行转义 #357

Closed
wants to merge 4 commits into from
Closed

Conversation

lusess123
Copy link

@coveralls
Copy link

coveralls commented Apr 20, 2018

Pull Request Test Coverage Report for Build 222

  • 0 of 0 (NaN%) changed or added relevant lines in 0 files are covered.
  • 112 unchanged lines in 5 files lost coverage.
  • Overall coverage increased (+0.002%) to 4.764%

Files with Coverage Reduction New Missed Lines %
packages/umi-plugin-dll/src/buildDll.js 6 0.0%
packages/umi-build-dev/src/getPlugins.js 10 0.0%
packages/umi-build-dev/src/getWebpackConfig.js 11 0.0%
packages/umi-build-dev/src/Service.js 32 0.0%
packages/umi-build-dev/src/FilesGenerator.js 53 0.0%
Totals Coverage Status
Change from base Build 203: 0.002%
Covered Lines: 81
Relevant Lines: 1722

💛 - Coveralls

Copy link
Author

@lusess123 lusess123 left a comment

Choose a reason for hiding this comment

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

支持lerna HRM

@@ -68,9 +68,6 @@ export default function dev({
'access-control-allow-origin': '*',
},
publicPath: webpackConfig.output.publicPath,
watchOptions: {
Copy link
Member

Choose a reason for hiding this comment

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

这个删了会不会让 watch 有性能问题?

@@ -424,14 +424,13 @@ export default function getConfig(opts = {}) {
},
{
test: /\.(js|jsx)$/,
include: opts.cwd,
include: [opts.cwd,/packages/],
Copy link
Member

Choose a reason for hiding this comment

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

默认不要加,有可能会误伤。通过环境变量开启吧,比如 LERNA=true 时才加上 /packages/ 的 include。

Copy link
Author

Choose a reason for hiding this comment

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

好的

Copy link
Member

Choose a reason for hiding this comment

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

想了想,这个通过 extraBabelIncludes 配就好,不要加这个逻辑了。

Copy link
Author

Choose a reason for hiding this comment

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

那tsx 怎么办? 这个不在babel配置里面啊

Copy link
Member

Choose a reason for hiding this comment

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

tsx 下面不是删了 node_modules 的 exclude 条件了吗?

Copy link
Author

Choose a reason for hiding this comment

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

哦,对 , 这个跟watch无关

Copy link
Author

Choose a reason for hiding this comment

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

我的意思是想让tsx 也应该有watch 功能,但是watch 因为性能问题忽略掉“node_modules”,我想是不是应该跟 babel 一样,也用白名单机制, 如果不能共用extraBabelIncludes配置,只能新增配置extraTsxIncludes 这样是否可行?

@sorrycc
Copy link
Member

sorrycc commented Apr 23, 2018

https://webpack.js.org/configuration/watch/#watchoptions-ignored
某些系统下 watch 大量文件会导致 CPU 占用问题。

@lusess123
Copy link
Author

@sorrycc 那就通过白名单的方式 添加watch 文件,其实主要是在lerna 上用

@sorrycc
Copy link
Member

sorrycc commented Apr 23, 2018

写个最简单的 lerna 例子,贴出来,我跑跑看?

@lusess123
Copy link
Author

@sorrycc 稍等

@lusess123
Copy link
Author

@sorrycc 重新修改测试了下,针对tsx , 发现只要注释代码:
// include: opts.cwd,
就OK了,并且也能进行HRM,
测试代码:
https://github.com/lusess123/lernaumi

@sorrycc sorrycc closed this Aug 17, 2018
sorrycc pushed a commit that referenced this pull request Jun 23, 2022
* ci: upgrade cache version

* ci: use pnpm action setup
xierenyuan pushed a commit to xierenyuan/umi that referenced this pull request Jun 23, 2022
* ci: upgrade cache version

* ci: use pnpm action setup
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.

None yet

3 participants