Skip to content

Common chunks#52

Merged
nighca merged 4 commits intoqiniu:masterfrom
doxiaodong:commonChunks
Jan 31, 2018
Merged

Common chunks#52
nighca merged 4 commits intoqiniu:masterfrom
doxiaodong:commonChunks

Conversation

@doxiaodong
Copy link
Copy Markdown
Contributor

先合并
#41

@nighca
Copy link
Copy Markdown
Collaborator

nighca commented Jan 11, 2018

注:
33c311d#diff-00d6a4118e3c6477c650be50d499891fR43

主要问题在这里,可能多 pages 的时候 两个page间的 async chunks 这里有一点点出入,不过我想了想好像对代码运行并无影响

@doxiaodong
Copy link
Copy Markdown
Contributor Author

doxiaodong commented Jan 11, 2018

同一个 page 有多个 entries: [1, 2] 的时候

如果 1 里面的两个 async chunks(记录为async1async2) 引入了 lodash,2 里面也引入了 lodash

则最后会在 2 里面有 lodash 的代码, 1 里面新增的 async chunks(记录为 async3) 也会有 lodash 代码

但是,通过实际运行服务发现,由于页面预先加载了 2 里面的 lodash ,之后便不在重新请求 async3 文件,所以效果符合预期

@doxiaodong
Copy link
Copy Markdown
Contributor Author

~

Comment thread README.md Outdated

- vendor

公用固定依赖集合,使用字段信息如 entries ,但是不支持数组
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

这个说明不太清晰啊,没看明白 vendor 的值需要是什么格式的;

使用字段信息如 entries

这里的 entries 不好确定说的是最外层的 entries 还是 pages 中的 entries

另外为什么 vendor 需要在每个 page 下指定呢,我感觉,vendor 跟 page 好像没啥关系?它跟 entries 有关系,是用来抽取 entries 间的公共部分的,所以好像整个项目只需要一处 vendor 的配置?

const _ = require('lodash')
const webpack = require('webpack')
const update = require('immutability-helper')
const { abs } = require('../../utils/paths')
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

return {
fixedName: fixPageName(name),
vendor: page.vendor,
entries: Array.isArray(page.entries) ? page.entries : [page.entries]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

entries.forEach(entry => {
vendorPlugins.push(
new webpack.optimize.CommonsChunkPlugin({
name: vendor,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

要不要指定一个带 hash 的 filename 啊?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

结果会自己带 hash 的啊

vendorPlugins.push(
new webpack.optimize.CommonsChunkPlugin({
name: vendor,
chunks: [
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

这是对 page 上的每个 entry,都创建一个它跟该页 vendor 的 common chunk?这事儿好像没啥收益啊

如果是按 page 配 vendor,为啥不是该页所有的 entry 与该 vendor 去创建一个 common chunk?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

因为 entry 和 entry 之间可能有相同部分,如果 vendor + entry1 + entry2 去抽取的话,会把 vendor 里没有 entry1, entry2 里没有的抽进 vendor 里。所以按照 vendor + entry1, vendor + entry2 这样抽

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

如果 vendor + entry1 + entry2 去抽取的话,会把 vendor 里没有 entry1, entry2 里没有的抽进 vendor 里

没太理解

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

会把 vendor 里没有 entry1, entry2 里有的抽进 vendor 里。多打了一个字

// 确保 vendor 在内容不变的情况下不会因为对 entries 的依赖改变而导致 hash 发生变化
...fixedPages.map(({ fixedName, entries, vendor }) => {
if (vendor) {
return new webpack.optimize.CommonsChunkPlugin({
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

这边 name 没有对应的 chunk,minChunks 又设置的 Infinity,所以其实会创建空的 common chunk?为啥这个可以:

确保 vendor 在内容不变的情况下不会因为对 entries 的依赖改变而导致 hash 发生变化

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

因为在 页面进行改动的情况下,内部的 requireid 可能会重新分配,如果没有这个的话,会导致 vendor 里面的 requireid 也跟着变化。 有了这个文件后,vendor 里的不会变化,变化的是这个接近空的文件。 https://webpack.js.org/plugins/commons-chunk-plugin/#manifest-file

Copy link
Copy Markdown
Collaborator

@nighca nighca Jan 24, 2018

Choose a reason for hiding this comment

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

明白,为啥我们不按惯例叫 "manifest",这边要防止名字冲突的话可以带上类似 fec 的前缀,__ 感觉不是特别好看 🤣

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

我老是 把 manifest 和 manifest.json 联系在一起,再加上那个 ssr 也生成了一个 manifest

plugins: { $push: [
// async common-chunks
// 这里指定的是全部 pages 因为 chunks 和 children 不能同时设定
new webpack.optimize.CommonsChunkPlugin({
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

对,这个是针对 code-splite 做的,就是 async-chunk 的那些也有公共部分

}),
// 确保 vendor 在内容不变的情况下不会因为对 entries 的依赖改变而导致 hash 发生变化
...fixedPages.map(({ fixedName, entries, vendor }) => {
if (vendor) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

我看这里有根据 vendor 进行筛,上边 vendorPlugins 也有做这个事儿,可以提前把 fixedPages
根据是否有 vendor filter一下,得到 fixedPagesWithVendor,然后这两处用 fixedPagesWithVendor map 得到就好了

return null
})
.filter(item => item !== null),
// 确保 entries 中所有在 vendor 中引入的依赖不会再被重复引入
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

确保 entries 中所有在 vendor 中引入的依赖不会再被重复引入

这个是怎么确保的。。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

这个问题好呀,我猜是他内部发现前面内存里已经分配了对应的 requireid , 所以这里遇到就不会重复做了,这个注释写得不好

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

嗯,我主要是不太确定具体是哪部分代码的行为,做了这行注释说的事情,因为这行注释后边的代码是:

...vendorPlugins,

没看出来“确保”的努力。。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

应该是我先写的注释,然后把代码挪走了,哈哈

Comment thread lib/utils/index.js Outdated
}

/**
* @desc 在设置 common-chunks 是使用, 调整 pageName, 使得 chunks 里不含 /
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

我看这个结果都是拿去拼 common chunk plugin 的 name 的,这里边含 / 是会有啥后果嘛?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

没啥影响,只是 static 里面多一层目录,感觉不好看

@nighca
Copy link
Copy Markdown
Collaborator

nighca commented Jan 23, 2018

整体感觉是,针对每个 page 配好像收益不大,但是理解配置的难度会上升一些。如果不针对每个 page,而是对整个项目的 entries 做,逻辑会不会简单一点?

@doxiaodong
Copy link
Copy Markdown
Contributor Author

这样其实差不多,都要配置三个 common-chunks, 而且也是需要指定 chunks 的,因为 vendor 不能按全部 chunks 抽离

@nighca
Copy link
Copy Markdown
Collaborator

nighca commented Jan 24, 2018

都要配置三个 common-chunks

其实是四个 1. manifest 2. async 3. vendor 4. common 对吧?

1、2 是不用用户配的,3 需要用户配,4 也不需要用户配(我们现在是每个页面一个 common);这样的话,我的想法是:

  • 4 可以改成针对项目中所有的 entries 抽,因为通常一个页面多个 entry 的情况很少,更多的情况是多页面多 entry
  • 3 的话,因为我们可以合理假定同一项目中的不同页面的技术方案是一致的,所以它们对 vendor 的需求应该也是一致的,我倾向于只配一处

因为 vendor 不能按全部 chunks 抽离

这是什么原理,不是不指定 chunks,就相当于所有 chunks 都使用了嘛?

@nighca
Copy link
Copy Markdown
Collaborator

nighca commented Jan 24, 2018

  1. vendor 不用这么复杂
  2. 跨整个项目
  3. 顶级的 "vendor"

@nighca
Copy link
Copy Markdown
Collaborator

nighca commented Jan 25, 2018

早上想到,抽 common 这个事情最好也是有个开关的,比如 { "extract-common": true },这样的配置,好像放到 optimization 里就比较合理了;然后就想到,vendor 的配置,是不是也放到 optimization 里?

大概就是这样了:

{
  "optimization": {
    "extract-common": true,
    "extract-vendor": "vendor"
  }
}

你看咋样

@doxiaodong
Copy link
Copy Markdown
Contributor Author

没啥问题

@doxiaodong
Copy link
Copy Markdown
Contributor Author

我发现 common 可以身兼 manifest 的功能

if (optCommon || optVendor) {
plugins.push(
new webpack.optimize.CommonsChunkPlugin({
name: chunks.common
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

这里不用指定个 minChunks 嘛?比如先用 2 或 entries 的数量?没查到不指定的话会发生什么。。

Copy link
Copy Markdown
Contributor Author

@doxiaodong doxiaodong Jan 31, 2018

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍

Comment thread README.md Outdated
import 'lodash'
// ...
```
注:如果设置了改自动,则 extract-common 也开启
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

我觉得这个绑定的行为不太合理啊,我指定要抽 vendor,不代表我想抽 common 吧?

好像这里绑定起来主要是为了利用 common 作为 manifest?那是不是也开了 extract-common 的时候就利用它,没开的时候就创建一个 manifest?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

common 这个设置为不可更改,就是都开启?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

不是啊,开启 common 的时候就搞一个 common chunk,顺便可以当 vendor 的 manifest 使;没开启 common 的时候,如果开了 vendor,vendor 就自己去搞个 manifest 的 chunk?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

我是说 common 不开启没好处啊

Copy link
Copy Markdown
Collaborator

@nighca nighca Jan 31, 2018

Choose a reason for hiding this comment

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

common 不开启就享受不到抽 common 的好处,这个是预期的呀;如果开了 vendor,不开 common,那我们帮他搞个 manifest + vendor,抽 vendor 的好处不是还是有的嘛?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

所以我说不抽没有好处啊。

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

手动关了 common,享受不到 common 的好处,不是预期的嘛。。如果手动关了 common,因为手动开了 vendor,导致 common 的行为被打开了,这个才不符合预期呀

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

所以我想把 common 从 optimization 里删掉啊

Copy link
Copy Markdown
Collaborator

@nighca nighca Jan 31, 2018

Choose a reason for hiding this comment

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

不要删掉吧,虽然说抽 common 有好处,但也有不好的点,我觉得还是要给关的机会的

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok

Comment thread README.md Outdated

优化项

- extract-common
Copy link
Copy Markdown
Collaborator

@nighca nighca Jan 31, 2018

Choose a reason for hiding this comment

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

这里是我的锅,我突然发现 build config 里其他的字段名都是驼峰的,这里是不是也驼峰好一点

Comment thread README.md Outdated

- extract-common

是否处理 common-chunks
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

“是否抽取 entries 间的公共内容到单独的文件中”?

Copy link
Copy Markdown
Collaborator

@nighca nighca left a comment

Choose a reason for hiding this comment

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

👝

@nighca nighca merged commit 209f530 into qiniu:master Jan 31, 2018
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.

2 participants