Skip to content

Conversation

@bebraw
Copy link
Contributor

@bebraw bebraw commented Feb 16, 2017

What kind of change does this PR introduce?

Feature.

Did you add or update the examples/?

Yes.

Summary

Now you can use

{
  overlay: {
    errors: true,
    warnings: true
  }
}

Closes #789.

Does this PR introduce a breaking change?

No.


The script should open the browser and show a heading with "Example: overlay with warnings".

In `app.js`, make a syntax error. The page should now refresh and show a full screen error overlay that shows the syntax error.
Copy link
Member

Choose a reason for hiding this comment

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

Could you document how to trigger a warning here? This text only tells me how to trigger an error. Would be handy for testing, since I can't come up with something that triggers a warning out the top of my head right now :).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the problem is, how to trigger a warning.

I tried copying

// This results in a warning:
// if(!window) require("./" + window + "parseable.js");

to the example from modus-inline, but that doesn't trigger a warning at all (not there either). Maybe something changed between webpack 1 and 2?

Copy link
Member

Choose a reason for hiding this comment

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

Jup, in webpack 1 that triggered a warning but in webpack 2 not anymore :'). Looking at webpack's source code on warnings.push, I found that it should trigger a warning when incorrectly cased modules are used. So perhaps add a file caseSensitive.js and import it with casesensitive.js?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is that those two files resolve fine on macOS. It would fail only on Windows and so on.

Copy link
Member

@SpaceK33z SpaceK33z Feb 17, 2017

Choose a reason for hiding this comment

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

Yes, but isn't the purpose of the CaseSensitiveModulesWarning plugin to give a warning for that, exactly because it works on macOS but not on Linux/Windows? (I seriously don't know btw, I only looked at the code very quickly 😄 )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only obvious way I can see would be to go through eslint and eslint-loader. That would work regardless of the platform.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yeah. I see now. I think I have to add the plugin to the configuration as it's not enabled by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. You have to do

require("./casesensitive.js");
require("./caseSensitive.js");

It requires multiple, different imports pointing to the same place in order to work. This is enough for me. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@bebraw bebraw force-pushed the feat-overlay-warnings branch from 0eb66ff to a347cde Compare February 17, 2017 11:00
Now you can use

{
  overlay: {
    errors: true,
    warnings: true
  }
}

Closes webpack#789.
@bebraw bebraw force-pushed the feat-overlay-warnings branch from a347cde to 3d8ccd5 Compare February 17, 2017 11:12
@SpaceK33z SpaceK33z merged commit 4991704 into webpack:master Feb 18, 2017
dcarral added a commit to dcarral/webpack-book that referenced this pull request Apr 22, 2017
Implemented @ webpack/webpack-dev-server#790,
'overlay: true' is equivalent to 'errors: true' && 'warnings: true'
dcarral added a commit to dcarral/webpack-dev-server that referenced this pull request Apr 22, 2017
This change tweaks the feature behavior (implemented @ webpack#790),
so it matches the intended behavior (described @ webpack#789).

This PR would supersede survivejs/webpack-book#262.
shellscape pushed a commit that referenced this pull request Jul 10, 2017
This change tweaks the feature behavior (implemented @ #790),
so it matches the intended behavior (described @ #789).

This PR would supersede survivejs/webpack-book#262.
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