benwiley4000 / cassette

📼 A flexible media player component library for React that requires no up-front config
https://benwiley4000.github.io/cassette/styleguide
MIT License
185 stars 28 forks source link

Is array-find-index polyfill necessary? #319

Closed danielr18 closed 5 years ago

danielr18 commented 5 years ago

https://www.npmjs.com/package/array-find-index https://github.com/benwiley4000/cassette/blob/fa9c48fdc13009972a80f54ca227d68c807d3cef/packages/core/src/utils/findTrackIndexByUrl.js#L1

From what I can see in the webpack config for core, @babel/preset-env preset is used with targets: undefined.

Sidenote, if no targets are specified, @babel/preset-env behaves exactly the same as @babel/preset-es2015, @babel/preset-es2016 and @babel/preset-es2017 together (or the deprecated babel-preset-latest).

Wouldn't Array.prototype.findIndex be transpiled by babel?

benwiley4000 commented 5 years ago

Good question!

Array.prototype.findIndex is part of the standard library, not the syntax, so it needs to be polyfilled. Sometimes people use babel-polyfill for this (which is different than the babel syntax transform).

Sooner or later I need to do an inventory of all the newer web platform features I'm using and figure out which ones actually need to be polyfilled and let people bring their own polyfills for the rest. But I think that would be a separate issue.

benwiley4000 commented 5 years ago

Are we ok to close this issue? Please let me know if you think I overlooked something. :)

danielr18 commented 5 years ago

Thanks for the answer, yes, it's babel-polyfill that takes care of that.

If the core-js package were used, the library could use the native implementation if it's present or polyfill it if it's not, doing something like import 'core-js/fn/array/find-index';, and that way you would more easily know which polyfills you use (for an inventory later on) by just looking for core-js.

But that would pollute global namespace and you as library author might not want to do that, in that case, I think core-js can also be used to import some polyfills as functions, that way there's no pollution but keep the perk of using just one package for pollyfills and make them easier to track.

benwiley4000 commented 5 years ago

Yeah, for things that really need to be monkeypatched on I would prefer asking folks to bring their own polyfill.

But I can definitely see the appeal of using a single package for polyfills. Currently the only ones I think I'm using are resize-observer-polyfill and array-find-index, and ResizeObserver isn't part of core-js, so I think I'll keep it how it is for now. But if I start to realize I need to polyfill lots of stuff that's part of ECMAScript I might take your suggestion! Thanks!

benwiley4000 commented 5 years ago

Btw there are some other things that need to be polyfilled in certain cases - requestAnimationFrame, cancelAnimationFrame, Map and Set - but those are already dependencies of React 16 so I'm assuming they exist in the global scope.