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

Wait until apply to create inspectpack daemon #215

Merged
merged 1 commit into from
Nov 7, 2017

Conversation

tschaub
Copy link
Contributor

@tschaub tschaub commented Oct 27, 2017

This makes it so the dashboard plugin can be configured without creating the inspectpack daemon. If the daemon is created in the constructor, the plugin cannot be conditionally applied. Currently, if the plugin is configured and added as part of a development only config, the process running the production config will never exit. With the changes here, the plugin can be configured but if it is never applied, the webpack process exits properly.

Fixes #96.
Fixes #125.

@tschaub
Copy link
Contributor Author

tschaub commented Oct 27, 2017

For people running into the "webpack never exits if the dashboard plugin is configured" issue, a workaround is to lazily construct the plugin.

For example, in your webpack dev config, instead of creating the plugin and adding it to your plugins array, you can add an object with an apply function that lazily constructs the plugin like this:

// webpack.config.js
const DashboardPlugin = require('webpack-dashboard/plugin');

const devConfig = {
  // rest of config as usual...
  plugins: [
    {
      apply: function() {
        const dashboard = new DashboardPlugin({color: 'cyan'});
        dashboard.apply.apply(dashboard, arguments);
      }
    }
  ]
};

Copy link
Member

@ryan-roemer ryan-roemer left a comment

Choose a reason for hiding this comment

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

Approach looks good in general.

Can you also stick a null check / guard in cleanup() in the weird case that this.inspectpack is unset?

@tptee -- can you take a look at this one?

Copy link
Contributor

@tptee tptee left a comment

Choose a reason for hiding this comment

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

👍

@tschaub
Copy link
Contributor Author

tschaub commented Oct 29, 2017

@ryan-roemer I pushed a new commit so cleanup() guards against no this.inspectpack.

Copy link
Member

@ryan-roemer ryan-roemer left a comment

Choose a reason for hiding this comment

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

Looks good!

@ryan-roemer ryan-roemer merged commit 839fbde into FormidableLabs:master Nov 7, 2017
@ryan-roemer
Copy link
Member

Released in webpack-dashboard@1.0.1. Thanks @tschaub !!!

@ryan-roemer ryan-roemer mentioned this pull request Nov 7, 2017
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.

None yet

3 participants