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

Display an overlay for syntax errors #89

Closed
davidascher opened this issue Jul 22, 2016 · 15 comments · Fixed by #744
Closed

Display an overlay for syntax errors #89

davidascher opened this issue Jul 22, 2016 · 15 comments · Fixed by #744
Milestone

Comments

@davidascher
Copy link
Contributor

My dev flow is to edit the code, and thanks to hot reload, look at the browser window to see the effect. I love those dev tools that in dev mode show the equivalent of what create-react-app currently dumps on console into the browser, e.g. when there are syntax errors.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

I agree we should print the terminal output in an overlay if the compilation fails.

@glenjamin worked on https://github.com/glenjamin/webpack-hot-middleware that does this, but IIRC it had some other problems.

So I’d like to see this as a feature, but probably implemented as a custom client instead of the one that ships with WebpackDevServer.

@glenjamin
Copy link

I'd happily accept any PRs / specific fixes for problems in hot-middleware, it already has a custom HMR client and is widely used.

You are of course free to port/fork, but it'd be good to share if possible.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

Could you make it not depend on polyfill in IE? As I recall this was the biggest issue.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

(Not saying it’s the only one tho. If you could make a PR porting from WDS to the middleware it would be easier to find other issues.)

@goshacmd
Copy link
Contributor

One thing I love about Brunch is that in case of error it will show a system notification, no config required. :w and seeing an error notification, it's easier to go to another terminal tab and see the stack trace, than to cmd+tab to the browser after saving a few files only to then spot an error.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

Sounds interesting. Perhaps we should do that. Want to take a stab at PR?

@goshacmd
Copy link
Contributor

There even seems to be a webpack plugin for that already https://www.npmjs.com/package/webpack-error-notification

@goshacmd
Copy link
Contributor

Which doesn't work on Windows, though.

Sure, I'll take a stab at it tomorrow!

@glenjamin
Copy link

The options for this are either plain WebSocket, which is IE10+ or a lib that magically deals with fallbacks - which would be weightier.

Any thoughts on what is preferable?

On 22 Jul 2016, at 20:31, Dan Abramov [email protected] wrote:

Could you make it not depend on polyfill in IE? As I recall this was the biggest issue.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

What was the motivation behind not using WebSockets?

@glenjamin
Copy link

Ironically I thought it'd be simpler & more reliable!

On 22 Jul 2016, at 23:17, Dan Abramov [email protected] wrote:

What was the motivation behind not using WebSockets?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

goshacmd added a commit to goshacmd/create-react-app that referenced this issue Jul 23, 2016
Uses webpack-notifier under the hood. No setup necessary.
Supports Notification Center on OS X, Toaster on Windows 8 and later.
Falls back to Growl if present.

Ref facebook#89.
@goshacmd
Copy link
Contributor

(Submitted #131 to add notifications)

@gaearon gaearon changed the title In dev mode, display the (beautiful) error messages in the browser window Display an overlay for syntax errors Sep 17, 2016
@gaearon gaearon added this to the 1.0.0 milestone Sep 17, 2016
@gaearon
Copy link
Contributor

gaearon commented Sep 17, 2016

I have a proof of concept of this working: https://twitter.com/dan_abramov/status/773799004878565376.
I’d like #419 to land first, and then I can add this.

@vjeux
Copy link
Contributor

vjeux commented Sep 24, 2016

#419 landed :)

@gaearon
Copy link
Contributor

gaearon commented Sep 24, 2016

I was just looking into it. 😄

@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants