Skip to content

fix(routes): Allow HMR with warnings#106

Merged
shellscape merged 1 commit intoshellscape:masterfrom
artembatura:fix/routes
Feb 5, 2019
Merged

fix(routes): Allow HMR with warnings#106
shellscape merged 1 commit intoshellscape:masterfrom
artembatura:fix/routes

Conversation

@artembatura
Copy link

This PR contains:

  • bugfix
  • feature
  • refactor
  • tests
  • documentation
  • metadata

Breaking Changes?

  • yes
  • no

If yes, please describe the breakage.

Please Describe Your Changes

We have two message types, which contains information about problems when Application building. There are warnings and errors. Currently when error or warning are detected, webpack-plugin-serve doesn't replace changed files. This PR allow apply changes to files by HMR if get any warning, because warning it's not critical problem, which affect on build process. Warnings never stopping building process. For example, warning from ESLint (eslint-loader)

@artembatura artembatura mentioned this pull request Feb 3, 2019
4 tasks
socket.lastHash = hash;

const { errors = [], warnings = [] } = stats.toJson(statsOptions);
const hasErrors = Boolean(errors.length);
Copy link
Owner

Choose a reason for hiding this comment

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

curious about this assertion. we're only in need of a truthy/falsy value here. was there an explicit need to case to Boolean?

Copy link
Author

@artembatura artembatura Feb 4, 2019

Choose a reason for hiding this comment

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

Yeah, this doesn't change much using Boolean. It's more stricter perhaps ^o^
And next I simply reuse value

Instead of this:

if (errors.length || warnings.length) {
  ...
  if (errors.length) return;
}

I written:

const hasErrors = Boolean(errors.length);
if (hasErrors  || warnings.length) {
  ...
  if (hasErrors) return;
}

@shellscape shellscape merged commit dc1db27 into shellscape:master Feb 5, 2019
@shellscape shellscape mentioned this pull request Mar 19, 2019
18 tasks
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.

2 participants