Skip to content

Made socket listener optional (required when using dev-middleware only) - #34

Merged
gregberge merged 6 commits into
gregberge:masterfrom
swernerx:feat/optional-socket-usage
Mar 21, 2019
Merged

Made socket listener optional (required when using dev-middleware only)#34
gregberge merged 6 commits into
gregberge:masterfrom
swernerx:feat/optional-socket-usage

Conversation

@swernerx

Copy link
Copy Markdown
Contributor

This PR makes the socked client optional which was used before for some more detailed error reporting. Unfortunately, this socket is only available when using webpack-dev-server and not webpack-dev-middleware.

The PR splits the entries for a basic integration and the socket listener into two. It then only adds the socket listener entry whenever there is a devServer config entry in Webpack.

The PR restores the previous behavior of v0.16.x for non-dev-server users.

Comment thread src/index.js Outdated

if (compiler.options.mode !== 'development') return

const enableDevServer = !!compiler.options.devServer

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

enableDevServer is an action, please rename it to devServerEnabled.

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.

Done 👍

@gregberge
gregberge merged commit 90caf91 into gregberge:master Mar 21, 2019
@gregberge

Copy link
Copy Markdown
Owner

Thanks!

@swernerx
swernerx deleted the feat/optional-socket-usage branch March 21, 2019 16:23
@swernerx

Copy link
Copy Markdown
Contributor Author

You are welcome!

@bencooper222

Copy link
Copy Markdown
Contributor

Is this a breaking change? It doesn't really look like one.

@gregberge

Copy link
Copy Markdown
Owner

No it is not.

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.

3 participants