mattkrick / redux-optimistic-ui

a reducer enhancer to enable type-agnostic optimistic updates
MIT License
693 stars 36 forks source link

Don't require Object.assign polyfill for older browsers #14

Closed wdoug closed 7 years ago

wdoug commented 8 years ago

In our environment we don't have an Object.assign polyfill, and it would be great (and simple) to not have this library require it.

As I see it, probably the simplest option is to use object spread in the source instead of Object.assign. For example, this:

Object.assign({}, actionToCommit[1].meta,
    {optimistic: null})
})

would become:

{
    ...actionToCommit[1].meta,
    optimistic: null
}

Another option would be to use something like the runtime transform which will transpile Object.assign for you.

Wanted to reach out and discuss before opening a Pull Request.

mattkrick commented 8 years ago

There was a light discussion about this in #10. Is there any reason you'd be against putting an Object.assign polyfill in your application code?

wdoug commented 8 years ago

There are some conflicting issues that we have with some legacy code if we bring in the babel polyfill, and there are a few other complications with just bringing in an Object.assign polyfill, mostly though it seems silly that such a small library requires us to bring in another polyfill dependency.

Is there any reason that you would be opposed to just using object spread?

mattkrick commented 8 years ago

The object spread just bakes the polyfill into the code:

var _extends = Object.assign || function (target) { for (var i = 1; i < arguments.length; i++) { var source = arguments[i]; for (var key in source) { if (Object.prototype.hasOwnProperty.call(source, key)) { target[key] = source[key]; } } } return target; };

So, the question is: do we put the polyfill in redux-optimistic-ui or in your application. I'd argue that it belongs in the application. That way, you won't have this problem with other packages, and adding a global polyfill to bring a browser up to compliance is always a good thing.

From a higher level view, we have a contract between code & client to use ECMAScript. My code keeps that contract, and older browsers do not. The party that breaks the contract should pay the price.

wdoug commented 8 years ago

Okay that's fair