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

Set options.host to 'localhost' #1617

Merged
merged 1 commit into from
Oct 6, 2017
Merged

Set options.host to 'localhost' #1617

merged 1 commit into from
Oct 6, 2017

Conversation

caryli
Copy link
Contributor

@caryli caryli commented Oct 4, 2017

Original discussion reference: webpack/webpack-dev-server#1129

Summary
Define the host property of the options object and set it to 'localhost'.

When host is not specified, host is defaulted to 'localhost' when using the webpack-dev-server CLI, but is left undefined when using the webpack-dev-server Node.js API with addDevServerEntrypoints.

Because of this, the code example will throw Uncaught SyntaxError: The URL 'http:/sockjs-node' is invalid in the browser.

There are differences between the CLI and Node.js API as discussed here: webpack/webpack-dev-server#616

Details
When using webpack-dev-server with the Node.js API in the following scenario:

  1. using addDevServerEntrypoints to inject webpack-dev-server into the entry point in webpack.config.js instead of manually (i.e. entry: ['./app.js', '../../client/index.js?http://localhost:5000/'])

  2. the options object passed as the argument to the devServerOptions parameter of addDevServerEntrypoints does not have a host property nor a useLocalIp property as in the below example used at https://webpack.js.org/guides/hot-module-replacement/#via-the-node-js-api:

    dev-server.js

    const webpackDevServer = require('webpack-dev-server');
    const webpack = require('webpack');
    
    const config = require('./webpack.config.js');
    const options = {
      contentBase: './dist',
      hot: true
    };
    
    webpackDevServer.addDevServerEntrypoints(config, options);
    

because of:

webpack-dev-server/lib/util/createDomain.js

11  const hostname = options.useLocalIp ? internalIp.v4() : options.host;

hostname will be set to options.host which is currently undefined.

This is not the case when using the webpack-dev-server CLI as defaults are set in webpack-dev-server/bin/webpack-dev-server.js. That step is skipped when calling addDevServerEntrypoints directly through the Node.js API.

Because of this, devClient:

webpack-dev-server/lib/util/addDevServerEntrypoints.js

18  const devClient = [`${require.resolve('../../client/')}?${domain}`];
19  console.log(devClient); // Added to print devClient.

will incorrectly resolve to and inject into webpack.config.js:

$ [ '/Users/someFolder/node_modules/webpack-dev-server/client/index.js?http:' ] // console.log(devClient)

instead of the correct:

$ [ '/Users/someFolder/node_modules/webpack-dev-server/client/index.js?http://localhost' ] // console.log(devClient)

This will eventually cause the error:

Uncaught SyntaxError: The URL 'http:/sockjs-node' is invalid

when starting the server:

$ node dev-server.js

and navigating to, in this case, localhost:5000 because socksjs-client is called with a url without a host property:

sockjs-client/lib/main.js

67  var parsedUrl = new URL(url);
68  if (!parsedUrl.host || !parsedUrl.protocol) {
69    throw new SyntaxError("The URL '" + url + "' is invalid");
70  }

The behavior of the webpack-dev-server CLI is different and correctly defaults to localhost:

$ ./node_modules/webpack-dev-server/bin/webpack-dev-server.js --content-base dist/
[ '/Users/someFolder/node_modules/webpack-dev-server/client/index.js?http://localhost:8080' ] // console.log(devClient)
Project is running at http://localhost:8080/
webpack output is served from /

because the options object passed as the argument to the devServerOptions parameter of addDevServerEntrypoints is given the defaults host: 'localhost' and port: 8080:

webpack-dev-server/bin/webpack-dev-server.js

 72  const DEFAULT_PORT = 8080;
...
 74  yargs.options({
...
210    host: {
211      type: 'string',
212      default: 'localhost',
213      describe: 'The hostname/ip address the server will bind to',
214      group: CONNECTION_GROUP
215    },
...
221  });
222
223  const argv = yargs.argv;
...
244  if (argv.host !== 'localhost' || !options.host) { options.host = argv.host; }
...
344  options.port = argv.port === DEFAULT_PORT ? defaultTo(options.port, argv.port) : defaultTo(argv.port, options.port);
345  if (options.port != null) {
346    startDevServer(webpackOptions, options);
...
359  function startDevServer(webpackOptions, options) {
360    addDevServerEntrypoints(webpackOptions, options);

Note
In the example, server.listen is given the 5000 and 'localhost' arguments:

dev-server.js

server.listen(5000, 'localhost', () => {
  console.log('dev server listening on port 5000');
});

But the above 'localhost' is passed to the server, webpack-dev-server/lib/Server.js, and not the client, webpack-dev-server/client/index.js, that is injected into the entry point and Uncaught SyntaxError: The URL 'http:/sockjs-node' is invalid will continue to be thrown.

Define the host property of the options object and set it to 'localhost'. The webpack-dev-server CLI defaults the host to 'localhost', but addDevServerEntrypoints from the webpack-dev-server Node.js API does not set a default and leaves it undefined, which will eventually throw "Uncaught SyntaxError: The URL 'http:/sockjs-node' is invalid" in the browser when following the code example.

Discussion reference: webpack/webpack-dev-server#1129
@skipjack
Copy link
Collaborator

skipjack commented Oct 4, 2017

@shellscape can you take a look at this? For some reason I can't request you for review, I may add you as a contributor on this project if you don't mind. That way I should be able to for all WDS related PRs going forward.

@shellscape
Copy link
Contributor

@skipjack I'd be honored, please do feel free to add me. I'll take a look...

Copy link
Contributor

@shellscape shellscape left a comment

Choose a reason for hiding this comment

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

This is a good change. While the examples in webpack-dev-server's repo reflect the necessary settings, the docs here do not.

@skipjack skipjack merged commit 3c5fff0 into webpack:master Oct 6, 2017
dear-lizhihua referenced this pull request in docschina/webpack.js.org Oct 17, 2017
* update /content/loaders & /content/plugins

* docs(api): minor fix in cli

Make minor fix in the CLI documentation to reflect this comment:

webpack#1577 (comment)

* docs(plugins): clarify and simplify `SourceMapDevToolPlugin` docs (#1581)

Try to resolve the confusion from #1556. Also update the options
section to clean things up a bit both for readability and grammar.

Resolves #1556

* docs(guides): update "external limitations" example in author-libraries (#1583)

* docs(plugins): update commons-chunk-plugin.md (#1560)

Changed example to declare the plugin within `plugins` property.

* docs(guides): rewrite shimming (#1579)

Rewrite the main sections of the guide to synchronize it with
the other core guides. Briefly mention the other utilities that
don't require full on examples (details can be found via links).

Closes #1258

* docs(guides): fix small issues in shimming (#1591)

* docs(guides): fix entry path in typescript (#1592)

* docs(guides): fix typos in production (#1584)

* fix(sponsors): update to reflect opencollective change (#1588)

OpenCollective's structure changed a bit. This address that
and makes a few other notable changes:

- add additional sponsors
- allow to merge additional sponsors to existing sponsors
- limit width of logos to 3 x height to disallow very wide logos
- format money values with commas

* docs(config): update configuration-languages (#1590)

Remove local type def from TypeScript snippet. The @types/node type 
definition module already declares `__dirname` type:

https://git.io/v5Dr9

* docs(guides): update hot-module-replacement (#1539)

Add an example to demonstrate using hot module replacement with 
`webpack-dev-server`'s Node.js API instead of within a normal
configuration file.

* docs(guides): update development.md (#1586)

The article was missing references to `clean-webpack-plugin`
in the `webpack.config.js` file and `package.json` files.

* docs(guides): update tree-shaking.md (#1589)

As a conclusion, I thought it would be a good idea to show case 
a high-level explanation of why it's called tree shaking. Once the 
user reads the guide and ends up with a high-level explanation 
the word tree shaking will stick to the user's head.

* docs(guides): update code-splitting (#1585)

Suggest solutions to the problem right away seems to be a 
better approach. Use similar text as was used here:

https://webpack.js.org/api/module-methods/#import-

* docs(plugins): update module-concatenation-plugin (#1565)

Add more information about optimization bailouts. Attempt to transfer 
content from the blog post to the official docs. I want to follow up with 
a PR for better bailout reasons. The hope is that the reasons will match 
those listed in the table.

* docs(api): fix type in compiler.md (#1601)

* docs(config): update output.md (#1541)

Clarify interactions between `libraryTarget` and `library`. The 
interactions between the two and how some of the sections were not
well explained, in some cases were incorrect, misleading or missing
information.

* docs(api): add concurrent usage warning in node.md (#1599)

* docs(guides): update environment-variables (#1549)

Add documentation about setting environment variables in CLI with 
`--env`. Link to webpack _CLI Environment_ section. Add additional 
usage to `--env` CLI docs.

* update /content/loaders & /content/plugins

* Swap ordering of pre/post loaders (#1602)

`pre` loaders run at the beginning, `post` loaders run at the end.

* docs(sponsors): update segment's donations (#1603)

* fix(fetch): fix some issues when parsing remote content (#1604)

The previous regex was a simpler approach to the lead in content
issue in fetched plugin/loader readmes, however it wasn't targeted
enough to correctly remove all lead in content. The new approach
should be a bit more stable but honestly the real fix here would be
so better standardize/enforce the readme template before including
these packages in the documentation site. I also think we can simplify
the template a bit to make the parsing of these readme easier.

Resolves #1600

* docs(guides): fix typo in code-splitting (#1606)

* docs(config): update `inline` recommendation in dev-server (#1540)

Update tip based on discussion in #1540.

* docs(config): update pfx description in dev-server (#1595)

Clarify the difference between using `pfx` via the command line and
in a configuration.

* docs(config): clarify stats option sort and default (#1596)

* docs(config): add before/after hook info to dev-server (#1608)

* docs(guides): fix typo in development (#1609)

* docs(guides): fix typo in production.md (#1610)

* docs(readme): fix typos (#1615)

* fix(markdown): fix issue with multiple pipes in a single cell (#1611)

Introduced better fix for pipes escaping in Markdown table cells.

* docs(guides): update hot-module-replacement (#1614)

Second argument of `server.listen` is a hostname, in this context, 
`'localhost'`. The third argument of `server.listen` is the success 
callback...

https://github.com/webpack/webpack-dev-server/blob/master/lib/Server.js#L552

* docs(guides): fix `onClick` instances in caching (#1613)

* docs(config): fix typo in module.md (#1618)

* docs(guides): fix grammar in getting-started.md (#1619)

* docs(config): add filename to index example (#1620)

* docs(guides): update wds options in hot-module-replacement (#1617)

Define the host property of the options object and set it to 'localhost'. The 
webpack-dev-server CLI defaults the host to 'localhost', but `addDevServerEntrypoints` 
from the webpack-dev-server Node.js API does not set a default and leaves 
it undefined, which will eventually throw "Uncaught SyntaxError: The URL 
'http:/sockjs-node' is invalid" in the browser when following the code example.

webpack/webpack-dev-server#1129

* fix(sponsors): opencollective api changed, add validation (#1622)

* docs(development): update how-to-write-a-loader.md (#1621)

Changes `_` to `.` to have correct code.

* update /content/loaders & /content/plugins

* update contributors

* docs(plugins): add deepChildren to commons-chunk-plugin.md (#1625)

* feat(sponsors): highlight latest sponsors/backers (#1628)

* refactor: update information architecture

This commit contains the following changes:

- Merge support and development into a coherent "Contribute" section.
- Flatten `/api/plugins` in preparation for flattened page `group`ing.
- Sort "Contribute" section and rewrite index page using "Why Support?".
- Simplify and organize antwar config as much as possible.
- Move `title` / `description` fields to the actual components.

BREAKING CHANGE: The route restructuring and content reorg
breaks some routes. Before this is merged, we should make sure
to add redirects for anything broken.

* refactor(navigation): update routes and styling

Simplify routing to reflect antwar and content changes and remove
extraneous links. Simplify styling to first improve the design, but also
to provide a lot more breathing room so less hacks are needed for
smaller page sizes. With the updated design we should have room
for "Voting" and "Analyze" links once those micro apps are in a
better place.

* fix: add link to starter kits

"Support" is now included in the "Contribute" section and already has
a link in the top navigation. The starter-kits page was previously hidden
from users, so this exposes it a bit more.

* refactor(navigation): update the navigation component

Make minor styling change for consistent gutter. Extract link data into
a separate JSON file. Update variable and parameter names for clarity.

* feat: allow page grouping within sections via `group` field

Add support for a new `group` field in each page's YAML frontmatter to
group pages together in an intuitive way. The benefit of this approach vs
directories is that changing a pages group within the same top-level section
won't affect its route and thus will not break links to it.

* style(page): align content better on larger screens

* docs: remove empty pages

* docs(contribute): add writers-guide and update release-process

* docs: sort the base pages more intuitively

* docs(api): update page names and grouping

* fix(mobile): clean up mobile sidebar data

* docs(api): clean up formatting and fix linting errors

* docs(api): fix markdown listing issues

* docs(contribute): fix markdown linting issues

* docs(api): rewrite the index page

* docs(api): minor formatting and cleanup

* docs: add redirects for all broken routes

The changes in #1612 break a significant amount of pre-existing URLs, this
commit adds redirects for everything I could see that would be broken.

* style(page): fix small padding issue

The padding here is useful for browsers that display a full on scrollbar
instead of an overlay scrollbar. We can continue to think about how
that space it utilized in future commits/prs.

* docs(guides): update production.md (#1629)

Modify the doc accordingly to the following tweetstorm where Dan 
Abramov & Sean Larkin kindly explained this to me.

https://twitter.com/TheLarkInn/status/917524723579338752

* fix(markdown): wrap inline code (#1634)

This change makes inline code blocks wrap so long lines will not 
be hidden outside paragraph.

Resolves #1633

* docs(guides): fix small typo excluding closing brace (#1637)

* docs(guides): fix typo in development.md  (#1638)

* docs(guides): update hot-module-replacement.md (#1639)

Maintain consistent code from previous guide (development).

* docs(guides): maintain consistent code in production.md (#1640)

* docs(guides): update typescript.md

Configure TypeScript to compile modules to ES2015 module syntax,
enabling webpack to resolve the `import` / `export` syntax on it's own.
With this included, the only thing necessary to enable _tree shaking_
is the inclusion of the `UglifyJSPlugin` (which I'm not adding to this
article as it is just centered on getting started with TypeScript).

Resolves #1627

* update /content/loaders & /content/plugins

* fix LinkDropdown

* 修复 npm 命令错误导致编译不成功的问题
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