-
-
Notifications
You must be signed in to change notification settings - Fork 3k
don't attempt to import growl in browser build #2938
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
Conversation
That's a good question -- if Growl doesn't work in browsers to begin with, we'll want to know for sake of determining how to handle the vulnerability fix too. |
(P.S. Thanks for the PR! If we can confirm that Growl shouldn't even be used in the browser, this update looks good to me.) |
I had a project were I needed notifications in browser. I ended up setting Also by reading the code I could see that the best place to put this overriding is |
@BuonOmo were you using webpack to package the js? My issue was that webpack was including the npm -sourced growl package automatically, it was not just a runtime configuration issue. |
@ahamid Yes I packaged the whole js with webpack, using the mocha-loader. I've even created a question on stack overflow about it to see if someone had encountered the same issue. |
@BuonOmo interesting, i wasn't aware of |
It is not very well documented, I'd suggest you to read the code in order to see what it does. Anyway, for my previous question: may I implement notifications in browser for mocha ? (@ScottFreeCode) |
You're certainly welcome to give it a try! I can't promise it will be evaluated promptly (we've already got a bit of a backlog of stuff to go through at the moment), but we'd be happy to at least look at it at some point and see what it's like, then decide what we want to do with it. As for browser-entry.js, I don't happen to know of anywhere better to handle it in Mocha's codebase off the top of my head, though someone else on the team might. On the other hand, it may be worth asking whether Growl would want browser integration like that (in which case we could just ensure Growl is usable in the browser and we'd get the functionality), provided we resolve the previously mentioned issue of supporting newer versions of Growl somehow. |
Ok thank I'll make a PR and reference it here. I don't think growl is supporting browser, but maybe we should name the option after Anyway, let's discuss this in the according PR :) |
I am a bot that watches issues for inactivity. |
I am pretty sure that #2962 preempts this. |
Does growl work in the browser? I had to add this in order to perform es6 imports in a webpack build.