Skip to content

docs(plugins): add hashed-module-ids-plugin.md and named-modules-plugin.md #1392

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

Merged
merged 15 commits into from
Jul 14, 2017
Merged

docs(plugins): add hashed-module-ids-plugin.md and named-modules-plugin.md #1392

merged 15 commits into from
Jul 14, 2017

Conversation

shaodahong
Copy link
Contributor

@shaodahong shaodahong commented Jul 7, 2017

add hashed-module-ids-plugin.md page #1384
and fix commons-chunk-plugin.md description

Resolves #1384

@shaodahong shaodahong changed the title add hashed-module-ids-plugin.md page Plugins: Add hashed-module-ids-plugin.md and named-modules-plugin.md Jul 11, 2017
Copy link
Contributor Author

@shaodahong shaodahong left a comment

Choose a reason for hiding this comment

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

Copy link
Collaborator

@skipjack skipjack left a comment

Choose a reason for hiding this comment

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

Thanks @shaodahong for documenting these plugins! Please see my minor requests.

- shaodahong
---

Will be based on the relative path of the module to generate a string of length 4 as the module id. Suggest for use in production.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably better phrased as:

This plugin will cause hashes to be based on the relative path of the module, generating a four character string as the module id. Suggested for use in production.

- shaodahong
---

The relative path of the module is displayed when [HMR](../guides/hot-module-replacement). Suggest for use in development.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably better phrased as:

This plugin will cause the relative path of the module is to be displayed when [HMR](/guides/hot-module-replacement) is enabled. Suggested for use in development.

@@ -159,4 +160,4 @@ module.exports = function(env) {
buildConfig
);
}
};
} : {};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are any changes required in this file? If anything I would recommend pushing these to a separate PR that describes why they're needed. I have a feeling this may have to do with the issue you created #1414, so we could discuss there as well.

@shaodahong
Copy link
Contributor Author

@skipjack I have changed

To generate identifiers that are preserved over builds, webpack supplies the `NamedModulesPlugin` (recommended for development) and `HashedModuleIdsPlugin` (recommended for production).

?> When exist, link to `NamedModulesPlugin` and `HashedModuleIdsPlugin` docs pages
To generate identifiers that are preserved over builds, webpack supplies the [`NamedModulesPlugin`](../plugins/named-modules-plugin) (recommended for development) and [`HashedModuleIdsPlugin`](../plugins/hashed-module-ids-plugin) (recommended for production).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use absolute URLs such as /plugins/named-modules-plugin

@@ -111,7 +111,7 @@ new webpack.optimize.CommonsChunkPlugin({
<script src="app.js" charset="utf-8"></script>
```

Hint: In combination with long term caching you may need to use the [`ChunkManifestWebpackPlugin`](https://github.com/diurnalist/chunk-manifest-webpack-plugin) to avoid that the vendor chunk changes. You should also use records to ensure stable module ids.
Hint: In combination with long term caching you may need to use the [`ChunkManifestWebpackPlugin`](https://github.com/diurnalist/chunk-manifest-webpack-plugin) to avoid that the vendor chunk changes. You should also use records to ensure stable module ids. ( e.g. using [`NamedModulesPlugin`](./named-modules-plugin) or [`HashedModuleIdsPlugin`](./hashed-module-ids-plugin) ).
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should also use records to ensure stable module ids. ( e.g. using [`NamedModulesPlugin`](./named-modules-plugin) or [`HashedModuleIdsPlugin`](./hashed-module-ids-plugin) ).
→
You should also use records to ensure stable module ids, e.g., using [`NamedModulesPlugin`](./named-modules-plugin) or [`HashedModuleIdsPlugin`](./hashed-module-ids-plugin).

Please use absolute URLs also here.

- shaodahong
---

This plugin will cause the relative path of the module is to be displayed when [HMR](/guides/hot-module-replacement) is enabled. Suggested for use in development.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo in "module is"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, what do you think?

Copy link
Collaborator

@simon04 simon04 Jul 13, 2017

Choose a reason for hiding this comment

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

"is" vs "ids"? "will cause the relative path of the module is to be displayed" does not seem to be correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean "will cause the relative path of the module to be displayed"

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah so I think you want to drop the "is".

... relative path of the module is to be displayed ...

to

... relative path of the module to be displayed ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is not a typo

Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo like in /typ/ing tree characters to/o/ much 😉

This plugin will cause hashes to be based on the relative path of the module, generating a four character string as the module id. Suggested for use in production.

``` js
new webpack.HashedModuleIdsPlugin()
Copy link
Collaborator

Choose a reason for hiding this comment

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

@skipjack
Copy link
Collaborator

@shaodahong thanks for making those updates and adding these pages! I did add one last commit to fix some grammar and formatting, so you may want to take a look at that -- but this should be good to ship :shipit:.

The build errors were just in relation to the edit links which is expected for new or renamed pages -- I'm going to let the build go through once more just in case and then I'll merge.

@skipjack skipjack changed the title Plugins: Add hashed-module-ids-plugin.md and named-modules-plugin.md docs(plugins): add hashed-module-ids-plugin.md and named-modules-plugin.md Jul 14, 2017
@shaodahong
Copy link
Contributor Author

@skipjack Thanks for helping me correct some grammar, some details are important, you are right

@skipjack skipjack merged commit 72552e9 into webpack:master Jul 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugins - Add HashedModuleIdsPlugin and NamedModulesPlugin
3 participants