Skip to content

Conversation

@mogsie
Copy link

@mogsie mogsie commented Dec 18, 2017

The notifier command fails with a message that browserSync.events is undefined; this is because the commands receive the emitter as the parameter, not the browserSync instance.

At least this change makes it possible to have tools run things like

curl 'http://localhost:3000/__browser_sync__?method=notify&args=Yeah<br><em>cool</em>!!'

:)

The notifier command fails with a message that browserSync.events is undefined; this is because the commands receive the emitter as the parameter, not the browserSync instance.

At least this change makes it possible to have tools run things like

    curl 'http://localhost:3000/__browser_sync__?method=notify&args=Yeah<br><em>cool</em>!!'

:)
@shakyShane
Copy link
Contributor

@mogsie Thanks for spotting this!

I couldn't apply your fix as there are too many places relying on the original implementation.

It was a mistake to sometimes pass the emitter, and sometimes the BS instance, but we'll have to wait for a major sem ver to fix

browser-sync/index.js

Lines 308 to 313 in 0aa00a7

exit: require("./lib/public/exit")(browserSync),
notify: require("./lib/public/notify")(browserSync),
pause: require("./lib/public/pause")(browserSync),
resume: require("./lib/public/resume")(browserSync),
reload: require("./lib/public/reload")(emitter),
stream: require("./lib/public/stream")(emitter),

@mogsie
Copy link
Author

mogsie commented Dec 25, 2017

Sure thing :) Maybe a new public API method could be put in place in order for notifications to work in the interim?

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