search

RoccoC / webpack-build-notifier

A Webpack plugin that generates OS notifications for build steps using node-notifier.
MIT License
162 stars 24 forks source link

Allow `notifyOptions` to depend on compilation status #70

Closed astorije closed 3 years ago

astorije commented 3 years ago

This allows for the following scenario: on success, clicking the notification should open the development URL (when running the dev server); on error, clicking the notification should open the terminal window. This is only possible if evaluating notifyOptions dynamically when calling notifier.notify instead of when the plugin is instantiated.

// webpack.config.ts
import WebpackBuildNotifierPlugin from 'webpack-build-notifier';

let address: string;

export default {
  // ... snip ...
  devServer: {
    // ... snip ...
    onListening(server) {
      const { port } = server.listeningApp.address();
      address = `http://localhost:${port}`;
    },
  },
  plugins: [
    // ... snip ...
    new WebpackBuildNotifierPlugin({
      notifyOptions(success) {
        return address && status === 'success' ? { open: address } : undefined;
      },
    }),
  ],
  // ... snip ...
};

This is particularly useful combined with #67 as it allows us to display Click here to open <address> on success only and use the default error/warning message otherwise. And combined with #68, it allows for nicely readable error/warning messages as well :)

astorije commented 3 years ago

I'm not sure why all these builds are failing. Travis status and Coveralls status are both 🟢 green, yet these return 500s. The other PRs were green yesterday, and when I rebased later on it started failing. It does seem like it's coming from one of the 2 vendors.

EDIT: Trying to open the Coveralls build on the latest master commit (which was green as well), there does seem to be a 500... https://coveralls.io/builds/37873324

EDIT2: Alright then https://github.com/lemurheavy/coveralls-public/issues/1543

EDIT3: Fixed!

coveralls commented 3 years ago

Pull Request Test Coverage Report for Build 162

Totals Coverage Status
Change from base Build 160: 0.1%
Covered Lines: 131
Relevant Lines: 135
💛 - Coveralls
astorije commented 3 years ago

@RoccoC, rebased on master and fixed conflicts!

RoccoC commented 3 years ago

Great, I will review this one shortly, thanks again!

astorije commented 3 years ago

@RoccoC, I rebased/fixed conflicts after your recent type consolidation changes, let me know if there is anything I can do to help, I'd love to be able to use this! :)

astorije commented 3 years ago

Hey @RoccoC, did you get a chance to look at this? :) Happy to help this project further!

RoccoC commented 3 years ago

Hey @astorije , thanks for the reminder -- and sorry, no, I haven't re-reviewed this yet, but will do so today! Thanks again for your contributions!

astorije commented 3 years ago

My pleasure and thank you, happy to help! :)

RoccoC commented 3 years ago

Merged and published in 2.3.0