Fixes #76. IE 11 now runs library out of the box

[dzzzchhh]

This PR introduces a small rework of a reduce function in createReducers.js that was using String.startsWith method that is not supported in IE 11, now it works fine out of the box.

[pauldijou]

I am kind of mixed about that. Going with this idea, libraries will never be able to use new standards and we will write only legacy code, which is sad. Also the question is up to which browser should we support? You want IE11, but what if someone else wanted IE9?

I think that it's up to the final user to decide which target (s)he wants to supports and include polyfills or use Babel to compile code that works on it.

Is there any issue with you putting that code as a polyfill inside your project?

[dzzzchhh]

@pauldijou I totally agree with you on overall state of supporting older browsers, however, in this particular case i want to point out 2 things:

1. IE 11 comes bundled with Windows 10, so some people may use it. Since this version (11) of IE comes with the most recent version of the mainstream OS, i think that it's worth considering the ability of the library to run in this particular browser. Older versions of Internet Explorer are out of the picture by now, in my opinion.

2. There is indeed an issue with using polyfills for my project, it has bundle size limitations and the issue with redux-act not supporting IE 11 is the only one that i am experiencing, so i kind of don't want to bring polyfills just for that issue.

PS: to summarize my point, i think it's crucial that a library should be able to complete its main goal without polyfills for relevant versions of browsers, which is IE 11, being part of Windows 10.

[pauldijou]

Sorry but I cannot agree with that. Even if IE11 is bundle with Windows 10, it is not the default browser. As far as I understand it, Microsoft has no plan to release a new major version of Windows any time soon but rather update the current one. So, going by your comment, libs would be stuck with IE11, which is years old, outdated and no longer updated, for just as much time. It has reached end of life as far as I am concerned.

That said, I agree that the lib should work on IE11 and it can if you include the needed polyfill. The startsWith polyfill is the exact same size as the added code in this PR so I'm pretty confident it will no be a bundle size problem. Also, rather than having each and every lib writing their own polyfill, adding more lines of code and potential bugs, you could rely on your own one.

Copy/pasting the one from MDN should do the trick in 5 lines of code.

[dzzzchhh]

Maybe add a hint to the README so that users won't be confused when they encounter this issue?

[pauldijou]

Sure I can do that. But is it ok for your project? Would feel a bit bad if that was the only reason preventing you from using it.

[dzzzchhh]

Well, it's clear that we have different opinions on using polyfills and who's responsible for library compatibility - author or developer that uses the library, and that is perfectly fine. It's just nice when things work out of the box, you know. Your library has proven itself very useful and it was a bit of a bummer when I found this issue.

I am still planning on using redux-act, just need to find a clean way to inject a polyfill that could've been avoided :-)

[pauldijou]

Done in 3ea7b0e

[ziir]

o/

Just a heads-up from long-time redux-act users, and unconditional fans :)

Release 1.6.0 which appears to have introduced the use of String.prototype.startsWith broke our production apps for some of our customers :(

We did not expect this to happen given that we had a similar discussion a while back in #48
Especially for such a trivial use-case.

One could argue that the following statement from the README is no longer true:
You can also use a browser friendly compiled file

We would have prefered not to, but we now polyfill String.prototype.startsWith in our apps, because we see how this view towards older browsers is gaining momentum in the npm ecosystem.

Of course, we also should have checked the changes before updating to 1.6.0.

Thanks again for everything.

bisous

[pauldijou]

First of all, sorry about that. But, to be honest, this makes me think about removing babel altogether actually. Final users, meaning devs using libs, should compile their full code, including node_modules, to whatever target they want to support.

Even if you did check the diff, what if it's a dependency of one of your dependency which introduce such problem? How deep are you checking? Don't trust 3rd party code to compile to the same prod env as the one you are targeting. I feel like having babel in this lib make people think "Oh, it's compiling to something which is probably safe enough for my use-case". And that's wrong unfortunately.

If you are not applying Babel to your node_modules currently in your project when releasing, please, considering doing it really soon combined with babel-env, this will ensure safe code for all of your users. (also, testing on targeted browsers would be nice but I don't do it either so, let's forget about that).

I will probably remove that line from the README and the compiled file. As far as I am concerned, this does not encourage best-practices. The goal of this file was to be included into online editors like JSBin, see #23, not as source of truth for production build.

[ziir]

TLDR: not to worry, we (users of redux-act) 'll be fine whatever you decide, as long as impactful decisions are documented and release with semver in mind.

While I'm not strictly opposed to having our whole node_modules go through babel, we simply do not need to at the moment.
There was this one time we considered doing it for a form handling library, but instead we simply decided to polyfill Set using the babel-runtime/core-js polyfill that's already in our bundle.
And another time with a data fetching library which relies on Promise, but that's about it until now.
Most libs I use compile to a set of ES5 that is safe enough for my use-case, unless specific case we handle specifically.
And in my opinion, this doesn't have much to with the authors using babel or not, it's more about documentation and on how to evaluate a library before adding it to a project.

Thank you for your suggestion, though we're not going down that road.
Even if we did have our whole node_modules go trough babel, we'd still be here discussing the use of String.prototype.startsWith because - unless I'm mistaken due to recent evolutions of babel things - babel-transform-runtime does not modify existing built-ins, meaning IE would still choke on String.prototype.startsWith. (using babel-polyfill is not an option for us)

All that said, I understand your point of view and I cannot object (as already discussed a while back in #48).
However, should you remove babel from this project, please consider it a breaking change and release it accordingly.

I believe you should keep both the compiled file and the mention in the README but give your users a piece of warning about production use, browser compatibility, etc.

Thanks again for your time, bisous.