Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Update Writing plugin and Plugin pattern page #2200

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
montogeek merged 7 commits into webpack:master from nveenjain:fix/writingPlugin
Sep 18, 2018

Conversation

Copy link
Member

@nveenjain nveenjain commented May 27, 2018

Fixes #2172, Updated writing plugins and plugin patterns page to include ES6 classes (as ES6 is supported by webpack completely) and updates related to new tapable library.

EugeneHlushko reacted with thumbs up emoji
Copy link
Member

Related to #2202.

Copy link
Member

@EugeneHlushko EugeneHlushko left a comment

Choose a reason for hiding this comment

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

Really good work here, but some stylistic and minor tweaks are requested, please pay attention to the rest of the changes where i didnt duplicate comments e.g. for ** -> __ and in javascript code blocks formatting (spaces, quotation, arrow funcs)

```
`applyPlugins(name: string, args: any...)`
It represents that the only hook supported is shouldEmit which is a hook of `SyncBailHook` type and the only parameter which will be passed to any plugin that taps into shouldEmit hook is compilation.
Copy link
Member

@EugeneHlushko EugeneHlushko Aug 21, 2018

Choose a reason for hiding this comment

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

`compilation`

Copy link
Member

@EugeneHlushko EugeneHlushko Aug 21, 2018

Choose a reason for hiding this comment

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

`shouldEmit`

## Different Plugin Shapes
A plugin can be classified into types based on the event it is registered to. Every event hook decides how it is going to apply the plugins in its registry.
A plugin can be classified into types based on the event hooks it taps into. Every event hook is pre-defined as synchronous/asynchronous/waterfall/parallel and hook is called internally using call/callAsync method. The list of hooks that are supported/can be tapped into are generally specified in this.hooks property.
Copy link
Member

@EugeneHlushko EugeneHlushko Aug 21, 2018

Choose a reason for hiding this comment

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

supported or can be tapped...

I would recommend to use or and comma separation here for the / synchronous/asynchronous/waterfall/parallel

// we've learned a lot about the source structure now...
class MyPlugin {
apply(compiler) {
compiler.hooks.emit.tapAsync("MyPlugin", (compilation, callback) => {
Copy link
Member

@EugeneHlushko EugeneHlushko Aug 21, 2018

Choose a reason for hiding this comment

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

we use single quotes for ```javascript i wont mark all of the places where you changed it or added as ", please change everywhere

apply(compiler) {
compiler.hooks.emit.tapAsync("MyPlugin", (compilation, callback) => {
// Explore each chunk (build output):
compilation.chunks.forEach(function(chunk) {
Copy link
Member

@EugeneHlushko EugeneHlushko Aug 21, 2018

Choose a reason for hiding this comment

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

we use arrow functions in webpack/webpack codebase in such blocks, i would recommend changing these into arrow functions for style consistency

return chunk.hash !== oldVersion;
}.bind(this));

class MyPlugin{
Copy link
Member

@EugeneHlushko EugeneHlushko Aug 21, 2018

Choose a reason for hiding this comment

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

the formatting is off, spaces are missing

### Synchronous Hooks
- __waterfall__ Plugins applied using
- **SyncHook**
Copy link
Member

@EugeneHlushko EugeneHlushko Aug 21, 2018

Choose a reason for hiding this comment

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

we use __ for boldness, please fix this and next occurrences

Here each of the plugins are called one after the other with the args from the return value of the previous plugin. The plugin must take the order of its execution into account.
It must accept arguments from the previous plugin that was executed. The value for the first plugin is `init`. This pattern is used in the Tapable instances which are related to the `webpack` templates like `ModuleTemplate`, `ChunkTemplate` etc.
In these type of hooks, each of the plugin callbacks will be invoked one after the other with the specific `args`. If any value is returned except undefined by any plugin, then that value is returned by hook and no further plugin callback is invoked. Many useful events like `"optimizeChunks"`, `"optimizeChunkModules"` are SyncBailHooks.
Copy link
Member

@EugeneHlushko EugeneHlushko Aug 21, 2018

Choose a reason for hiding this comment

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

You dont need to double quote the events, just `optimizeChunks`

- Called using `call( ... params)` method
`applyPluginsAsync(name: string, args: any..., callback: (err?: Error) -> void)`
Here each of the plugins are called one after the other with the args from the return value of the previous plugin. The plugin must take the order of its execution into account.
Copy link
Member

@EugeneHlushko EugeneHlushko Aug 21, 2018

Choose a reason for hiding this comment

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

args -> arguments

`applyPluginsAsync(name: string, args: any..., callback: (err?: Error) -> void)`
Here each of the plugins are called one after the other with the args from the return value of the previous plugin. The plugin must take the order of its execution into account.
It must accept arguments from the previous plugin that was executed. The value for the first plugin is `init`. Hence atleast 1 param must be supplied for waterfall Hooks. This pattern is used in the Tapable instances which are related to the `webpack` templates like `ModuleTemplate`, `ChunkTemplate` etc.
Copy link
Member

@EugeneHlushko EugeneHlushko Aug 21, 2018

Choose a reason for hiding this comment

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

Hooks -> hooks

Copy link
Member

@EugeneHlushko EugeneHlushko Aug 21, 2018

Choose a reason for hiding this comment

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

`webpack` -> webpack

- Called using `callAsync( ... params)` method
`applyPluginsAsyncWaterfall(name: string, init: any, callback: (err: Error, result: any) -> void)`
The plugin handler functions are called with all args and a callback function with the signature `(err?: Error) -> void`. The handler functions are called in order of registration. `callback` is called after all the handlers are called.
Copy link
Member

@EugeneHlushko EugeneHlushko Aug 21, 2018

Choose a reason for hiding this comment

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

args -> arguments

Copy link
Member

@EugeneHlushko EugeneHlushko left a comment

Choose a reason for hiding this comment

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

lgtm

@montogeek montogeek merged commit 19ead0e into webpack:master Sep 18, 2018
Copy link
Member

Thanks!

montogeek pushed a commit that referenced this pull request Sep 27, 2018 
* fix(contribute) Use consistent plugin name in code snippet (#2496)
* fix(configuration) Fix small typo (#2511)
* docs(manifest) Corrections and small update to concept manifest page (#2504)
Small updates and corrections for manifest concepts page
* docs(concepts) Update dependency graph concepts page (#2495)
* docs(concepts) Update dependency graph concepts page
* docs(concepts) Dependency graph minor tweak
* docs(concepts) Use common formatting, provide more links, mention mod... (#2472)
* docs(concepts) use common formatting, provide more links, mention mode in core of the concepts
* docs(concepts) duplicate links on the list for better ux
* docs(concepts) forgot to add EugeneHlushko to contributors
* docs(configuration) Add externals examples (#2515)
Update the external docs with an example on how to combine the different syntaxes. 
I felt it was not clear how to achieve this from the existing docs.
* docs(plugins) Update Writing plugin and Plugin pattern page (#2200)
* Update Writing plugin and Plugin pattern page
* Fixed formatting issues
* Fix markdown lint issues
* Update plugin-patterns.md
* Update plugin-patterns.md
* docs(concepts) Using vendors as entry is not recommended (#2525)
* Using vendors as entry is not recommended
fixes webpack/webpack#6647
* add note about vendor entrypoints
* fix(guides) Correct misspelling and punctuation errors (#2513)
* Correct misspelling and punctuation errors
* Update build-performance.md
* docs(plugins): Add custom vendor chunks using RegEx example (#2518)
* docs(config): Example 3 - custom vendor chunk using RegEx
* docs(config): Example 3 SplitChunks Fenced blank added
* docs(config): (typo) Example 3 - custom vendor chunk using RegEx
* sakhisheikh
docs(config): (reworded) Example 3 - custom vendor chunk using RegEx
* docs(config): (Reword) Example 3 - custom vendor chunk using RegEx
* docs(config): (Reword) Example 3 - custom vendor chunk using RegEx
* chore(plugins) Remove space (#2527)
* docs(config): Example 3 - custom vendor chunk using RegEx
* squashed everything after c81e77e
* docs(plugins) Add .mjs to the defaults (#2493)
This should be updated once webpack/webpack#7947 is released.
* chore(plugins) Minor SplitChunks Documentation Clean Up (#2252)
* Minor Documentation Clean Up
Originally, i was just trying to change defer => differ, but I ended up doing a bit more editing.
* Lower the case
* fix(api) Clarify module section (#2529)
Change wording from "bind an extension" to "bind a file extension" in order to distinguish from a webpack extension.
_describe your changes..._
- [ ] Read and sign the [CLA][1]. PRs that haven't signed it won't be accepted.
- [ ] Make sure your PR complies with the [writer's guide][2].
- [ ] Review the diff carefully as sometimes this can reveal issues.
- __Remove these instructions from your PR as they are for your eyes only.__
[1]: https://cla.js.foundation/webpack/webpack.js.org
[2]: https://webpack.js.org/writers-guide/
* docs(plugins): Update links in htmlwebpackplugin (#2530)
* docs(plugins) Fix reuseExistingChunk title nesting (#2526)
* docs(plugins) scp reuseExistingChunk title nesting fix
* docs(plugins) scp reuseExistingChunk title notation
* docs(guides) Fix typo (#2537)
* chore(guides) Import needs to be highlighted with '+' mark (#2536)
import webpack needs to be highlighted since we are adding webpack.HashedModuleIdsPlugin()
* docs(config) Optimization moduleIds option (#2543)
@nveenjain nveenjain deleted the fix/writingPlugin branch October 3, 2018 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EugeneHlushko EugeneHlushko EugeneHlushko approved these changes

+1 more reviewer

@dhruvdutt dhruvdutt dhruvdutt approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /