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

docs(config): Document how to use built-in Node.js modules #1368

Merged
merged 3 commits into from
Aug 6, 2017

Conversation

Rob--W
Copy link
Contributor

@Rob--W Rob--W commented Jul 5, 2017

The documentation about using built-in Node.js modules was very poor (i.e. non-existent). This PR expands the documentation of the "node" configuration option, and shows how one can require built-in modules if desired.

Furthermore, all possible effects of the options are explicitly documented. Instead of being vague of what happens when false is used, it is explicitly spelled out what happens.

References:


By default, Webpack will polyfill each library if there is a known polyfill or do nothing if there is not one.
T> To import a built-in module, use [`__non_webpack_require__`](/api/module-variables/#__non_webpack_require__-webpack-specific-),
Copy link

Choose a reason for hiding this comment

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

to import a built-in module

Instead of a polyfill, you mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also. Importing using the default require instead of a polyfill, mock, or erroring.

@@ -109,9 +120,18 @@ Default: `true`

`boolean | "mock" | "empty"`

Many other Node.js core libraries can be configured as well. See the list of [Node.js core libraries and their polyfills](https://github.com/webpack/node-libs-browser).
W> This option is only activated (via `NodeSourcePlugin`) when the target is unspecified, "browser" or "webworker".
Copy link

Choose a reason for hiding this comment

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

W>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This puts the text in a warning box. T> is a tip box, ?> is a TODO box.

@Rob--W Rob--W force-pushed the document-node-require branch from 570bb55 to f403a05 Compare July 6, 2017 10:51
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.

Sorry for the delay on this PR and thanks for your contribution. The changes look great for the most part, just added a few minor comments. Once those things are addressed we should be able to get this merged.

[`NodeStuffPlugin`](https://github.com/webpack/webpack/blob/master/lib/NodeStuffPlugin.js) plugin.
If the target is "web" (default) or "webworker", the
[`NodeSourcePlugin`](https://github.com/webpack/webpack/blob/master/lib/node/NodeSourcePlugin.js)
plugin is also activated.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you leave the formatting as is? We may automatically enforce a max line length at some point (webpack-contrib/webpack-defaults#73) but for now we typically keep paragraphs all on one line and encourage contributors to word-wrap while editing these files. That said, it probably makes sense to split it into 2 paragraphs as such:

These options configure whether to polyfill or mock certain [Node.js globals](https://nodejs.org/docs/latest/api/globals.html) and modules. This allows code originally written for the Node.js environment to run in other environments like the browser. 

This feature is provided by webpack's internal [`NodeStuffPlugin`](https://github.com/webpack/webpack/blob/master/lib/NodeStuffPlugin.js) plugin. If the target is "web" (default) or "webworker", the [`NodeSourcePlugin`](https://github.com/webpack/webpack/blob/master/lib/node/NodeSourcePlugin.js) plugin is also activated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Note that I did not just split the paragraph, I also added "plugin" after "NodeStuffPlugin", so that it has become "NodeStuffPlugin plugin".

- `false`: Provide nothing. Code that expects this object to be defined may crash.
- `false`: Provide nothing. Code that expects this object may crash with a `ReferenceError`.
Code that attempts to import the module using `require('modulename')` may
trigger a `Cannot find module "modulename"` error.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure this extra description is necessary. I see where you're going with it, but there could be any number of errors that occur so just specifying that it may crash should be enough.

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 added explicit error messages so that search engines can find the documentation more easily.

Polyfills for Node.js core libraries from
[`node-libs-browser`](https://github.com/webpack/node-libs-browser) are used if
available, when the `NodeSourcePlugin` plugin is enabled.
See the list of [Node.js core libraries and their polyfills](https://github.com/webpack/node-libs-browser#readme).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again, the changes here look 👍 but please drop the line breaks as it's all one paragraph.

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 combined the first three lines that formed one sentence in one line. The last sentence is still separate because the message that it conveys is not depending on the previous line.

See the list of [Node.js core libraries and their polyfills](https://github.com/webpack/node-libs-browser#readme).

By default, webpack will polyfill each library if there is a known polyfill or do nothing if there is not one.
In the latter case, webpack will behave as if the module name was configured with the `false` value.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


By default, Webpack will polyfill each library if there is a known polyfill or do nothing if there is not one.
T> To import a built-in module, use [`__non_webpack_require__`](/api/module-variables/#__non_webpack_require__-webpack-specific-),
i.e. `__non_webpack_require__('modulename')` instead of `require('modulename')`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here -- i.e. the whole T> should all fall on line 133.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@Rob--W Rob--W force-pushed the document-node-require branch from f403a05 to 21b1fc3 Compare August 4, 2017 10:15
The documentation about using built-in Node.js modules was very poor
(i.e. non-existent). This expands the documentation of the "node"
configuration option, and shows how one can require built-in modules if
desired.

Furthermore, all possible effects of the options are explicitly
documented. Instead of being vague of what happens when `false` is used,
it is explicitly spelled out what happens.

References:

- https://github.com/webpack/webpack/blob/a589a6c9789a9d342fc630e36ab81827dd20289b/lib/WebpackOptionsApply.js
  shows when the NodeStuffPlugin and NodeSourcePlugin plugins are used.
- https://github.com/webpack/webpack/blob/a589a6c9789a9d342fc630e36ab81827dd20289b/lib/NodeStuffPlugin.js
  is the plugin that is used for every target.
- https://github.com/webpack/webpack/blob/a589a6c9789a9d342fc630e36ab81827dd20289b/lib/node/NodeSourcePlugin.js
  is the plugin that is only used for "web" and "webworker" targets.
- https://github.com/webpack/webpack/blob/a589a6c9789a9d342fc630e36ab81827dd20289b/lib/MultiModule.js#L65-L67
  is the generated "webpackMissingModule" code for unresolved modules.
  (when the NodeSourcePlugin is not used, the environment's "require" is
  used. In Node.js, this also throws a "Cannot find module 'modulename'"
  error (with single quotes instead of double quotes)).
@Rob--W Rob--W force-pushed the document-node-require branch from 21b1fc3 to 6636c98 Compare August 4, 2017 10:16
@skipjack skipjack merged commit 1fdd547 into webpack:master Aug 6, 2017
dear-lizhihua referenced this pull request in docschina/webpack.js.org Aug 10, 2017
* docs(config): change `global` to `root` in externals  (#1214)

The currently deployed documentation indicates that there are 
4 module contexts you can specify under the externals config...

- `global`
- `commonjs`
- `commonjs2`
- `amd`

... which contradicts both the examples that surround it, and the 
actual library functionality. 

This just changes the word "global" to "root" in the section that lists 
the module contexts.

* docs(config): clarify that HMR plugin can be added via a flag (#1478)

* fix(markdown): resolve some odd code display bugs (#1486)

Fix list formatting in `/development/plugin-patterns/`. Remove the
`inline-block` declaration to let inline code flow more naturally
and prevent this odd wrapping if other badly formatted lists slip in.

* docs(guides): add note about chunkFilename (#1483)

* docs(config): clarify the meaning of `Rule.parser` options (#1489)

Fixes #1484

* docs(config): improve node documentation (#1368)

The documentation about using built-in Node.js modules was very poor
(i.e. non-existent). This expands the documentation of the "node"
configuration option, and shows how one can require built-in modules if
desired.

Furthermore, all possible effects of the options are explicitly
documented. Instead of being vague of what happens when `false` is used,
it is explicitly spelled out what happens.

References:

-
https://github.com/webpack/webpack/blob/a589a6c9789a9d342fc630e36ab81827dd20289b/lib/WebpackOptionsApply.js
  shows when the NodeStuffPlugin and NodeSourcePlugin plugins are used.
- https://github.com/webpack/webpack/blob/a589a6c9789a9d342fc630e36ab81827dd20289b/lib/NodeStuffPlugin.js
  is the plugin that is used for every target.
- https://github.com/webpack/webpack/blob/a589a6c9789a9d342fc630e36ab81827dd20289b/lib/node/NodeSourcePlugin.js
  is the plugin that is only used for "web" and "webworker" targets.
- https://github.com/webpack/webpack/blob/a589a6c9789a9d342fc630e36ab81827dd20289b/lib/MultiModule.js#L65-L67
  is the generated "webpackMissingModule" code for unresolved modules.
  (when the NodeSourcePlugin is not used, the environment's "require" is
  used. In Node.js, this also throws a "Cannot find module 'modulename'"
  error (with single quotes instead of double quotes)).

* docs(config): correct substitutions in output.md (#1487)

Correct `output.filename` substitutions and move the others to
`output.sourceMapFilename`. Also fix `related` link in the context-
replacement-plugin.

Fixes #1467
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants