Improve polyfill

[neftaly]

This PR prevents the polyfill from creating window.EventSourcePolyfill, and adds node support

[neftaly]

Is it better to create the window object in node, or attach to global instead?
eg if (typeof window !== object) global.window = {}

new EventSource vs new window.EventSource

[joeybaker]

hmmm… I'd say neither. In node, we'd probably just want to set module.exports. So, module.exports = EventSource. Take a look at the UMD spec: https://github.com/umdjs/umd

[neftaly]

I'm hoping to not have to do this in a module that's designed for both node & browser:

const EventSource = (typeof window === 'object' && window.EventSource)
  ? window.EventSource
  : require('eventsource');

Are you ok with something like this for eventsource-polyfill.js:

var EventSource = require('./eventsource')
if (typeof window === 'object') {
  if (!window.EventSource) window.EventSource = EventSource
  module.exports = window.EventSource;
} else {
  module.exports = EventSource;
}

That way, eventsource-polyfill becomes a commonjs module (that also polyfills the browser).

[joeybaker]

Seems fine in principle, but you'll probably want to add a check to make sure module is a valid thing before setting it :)

[neftaly]

True, I guess you have to cover for people doing <script src='eventsource-polyfill.js'> 😒

[neftaly]

Wait, no - you wouldn't need to, cause then require wouldn't work!

[joeybaker]

@rexxars This seems fine to me. I don't see the need for the additional global and this solves things in a nice way.

[piranna]

All these tricks are already done by browserify... El 21/6/2017 3:06, "Joey Baker" <notifications@github.com> escribió:

…

***@***.**** approved this pull request. @rexxars <https://github.com/rexxars> This seems fine to me. I don't see the need for the additional global and this solves things in a nice way. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#80 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAgfvlYySMSzpsQCcsFbG7lKiaIVaoXPks5sGGx_gaJpZM4OAT1f> .

[neftaly]

Ok, I'm using this in node on the backend and in webpack on the frontend however (both via ES6 import).

[rexxars]

I don't have any problems with the node compatibility, but I'd rather not break backwards compatibility at this point. If you reintroduced window.EventSourcePolyfill I will be happy to merge.

[neftaly]

Ping @rexxars

[rexxars]

Thanks for pinging and sorry for the delay. Released as 1.0.5.