use in a browser?

[getify]

From what I can tell, it seems like this should work in a browser. But I don't see a packaged file ready to load in a browser. Am I needing to use like webpack/rollup/etc to take this file (and the one from event-target-polyfill) and bundle them?

Would you consider an addition to add a build step so that the npm package has a distributable file ready to load in a browser as-is?

[benlesh]

What if we just had a check to see if the EventTarget was there? And then errored if it didn't exist?

[getify]

According to MDN, EventTarget has been around in the browser a long time. So I think it's plenty safe to skip loading in the browser.

However, IIUC, Node either doesn't have it yet, or just recently added it. That means it'd increase the cognitive load to use this polyfill in Node, in having to also manually load event-target polyfill. :/

[getify]

I have a tool Moduloze which can convert CJS modules to UMD (or ESM) as a build process.

I propose we could do this:

1. tweak the AC polyfill to omit the require(..) of EventTarget, and also have it expose itself like a normal CJS module with module.exports

2. move the AC polyfill to a separate file, then have the index.js file (the Node entry point) just have a couple of lines of logic check and forcibly load both the event target polyfill and then load the AC module and push it onto the global scope -- just for Node.

3. have the Moduloze build process build the CJS form of the AC polyfill into a UMD that will auto-install itself to the window/global scope.

TBH, the Moduloze part of this isn't strictly necessary... it just makes it a little cleaner and more robust. But if you didn't want to go that route, still separating into two separate files (as suggested in (1) and (2) above) would work.

And, I'm happy to PR for whichever route you think is best.

[benlesh]

EventTarget is not in all current versions of Node. Which is why I needed to add that.

I suspect that the best course of action is probably just to remove that import and have this rely on separate polyfill if necessary.

Then it's just a dumb JS file that can be loaded anywhere.

[getify]

For my purposes, I need an AbortController polyfill that works in both browser and Node... I had hoped to switch to this one. But if I did, and you separated them like that, then I'd have to manually bring in two polyfills.

So would anyone else who wanted to use this in Node, for any reason. That definitely makes the usage of this polyfill more difficult.

If you prefer to keep this simple and not have it come with a dist file that works in both node and the browser, then I'll have to evaluate if I can justify the complexity on my side, or if I'll stick to a different polyfill that doesn't have the dependency.

The offer stands, though, that I'm happy to PR (and would prefer it!) if you'd like an option to have this one provide a working solution OOTB, across both browser and server.

[benlesh]

I mean, the complexity of that is simply:

require('event-target-polyfill');
require('yet-another-abortcontroller-polyfill');

Yeah? So, really just one more line.

[benlesh]

I'm still thinking about it all, not really decided.

[benlesh]

I've published a 0.0.3 that removes the EventTarget polyfilling. Since we're firmly in the 0.0.x releases of this package, it's a worthy experiment. So we can at least try out the quick and easy solution. We'll see how the ergonomics of this work out. I'll leave this issue open for comment.

[getify]

I mean, the complexity of that is simply

It's true that it's "just" one extra line. Where I'm coming from (wanting to use this as a dependency in another OSS project: CAF), I don't want to tell users of my project that they need to figure out if they need to include not just one, but two different polyfills.

Right now, for browser usage, I distribute a single contained AC polyfill file in the dist/ folder, and I document for users that they may need it or not. I'm sensitive to those who really don't want to load any bytes they don't have to, so that's why it's in a separate file instead of just being included in the bundle for CAF.

For node usage, I have the central index.js file that includes the AC polyfill. But I know a number of people who bypass that because they dislike how I load the polyfill into the "global" scope, so they manually load in the polyfill and the src/ for CAF. So even if I add this second polyfill to that index.js logic, it means those who are bypassing it are now going to need to juggle two different polyfills.

Again, in the whole scheme of things, it's not that huge of a deal. But your decision to keep this lib simpler flows downhill to other projects (like CAF) that instead have to deal with the complexity. I'd rather not. I'd rather depend on a single lib that was self-contained.

Just my feedback on why I've gone to the trouble to make the suggestion.