Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What browsers does sprintf.js targets? #137

Closed
nazar-pc opened this issue May 18, 2017 · 18 comments
Closed

What browsers does sprintf.js targets? #137

nazar-pc opened this issue May 18, 2017 · 18 comments

Comments

@nazar-pc
Copy link
Contributor

What I'm actually curious about is whether you're fine with dropping IE entirely and only support last 2 versions of any evergreen browser in next major release?
If this is the case it would be possible to reduce and simplify code a bit.

@alexei
Copy link
Owner

alexei commented May 19, 2017

I haven't had access to any IE for a couple of years now so I don't really know what [still] works and what doesn't. Afaik IE > Edge so dropping it entirely is not really an option. Do note however the cache is based on a pure object so that means IE >= 11 I think. So then what optimizations are you thinking about? i.e. that would still include IE 11

@ArmorDarks
Copy link

According to our metrics, dropping IE definitely not an option so far, unfortunately.

@nazar-pc
Copy link
Contributor Author

For instance, all modern browsers (IE is not modern) support String.prototype.repeat(), so str_repeat() is not needed and native implementation should be faster (haven't tested yet). This was the first one I've noticed.

@ArmorDarks, I do not suggest deleting all of existing versions from Bower or NPM, you can use older version that supports IE as long as you need to. I just don't think that those who work with modern browsers should pull polyfills for something as simple as String.prototype.repeat() if their browsers have it built-in faster implementation. Also you can add String.prototype.repeat() polyfill for your project separately and use newer versions of this library, it just wouldn't be bundled anymore.

Don't you agree it is always easier to add stuff you need rather than remove something you don't need?

@alexei
Copy link
Owner

alexei commented May 19, 2017

Indeed one can use a polyfill for String.prototype.repeat() so I agree to using it.

I looked Object.create up on the Internet and noticed there are polyfills for it too. Though I'm not so sure they allow calling it with null as a parameter.

@alexei
Copy link
Owner

alexei commented May 19, 2017

@nazar-pc if you have suggestions likes this one, please open an issue for each one and mark them as enhancement or something. Or drop the list here and I'll create them myself.

@nazar-pc
Copy link
Contributor Author

I'll just look around for improvements and will create a PR with the list of changes, so we can discuss them concretely there.
Will close this issue for now.

@ArmorDarks
Copy link

Just don't forget to document all things, that should be polyfilled for IE. I imagine that digging into internals to find out which ones should will be quite not enjoyable ride...

@nazar-pc
Copy link
Contributor Author

Sure

@idubinskiy
Copy link

@alexei FYI, version 1.1.1's use of String.prototype.repeat broke one of our apps that was still using Node 0.12.

@nazar-pc
Copy link
Contributor Author

Node 0.12 is EOL for almost half a year now, you should really consider upgrade for security and performance reasons.
If that is impossible - just include String.prototype.repeat polyfill and it should work just fine.

@idubinskiy
Copy link

@nazar-pc We're actually in the process of upgrading our apps, including that one. However, it's still unpleasant to have a patch release include a change that broke the next build of our application.

@nazar-pc
Copy link
Contributor Author

Well, I think it would be useful to add a compatibility table for supported browsers and node.js versions. If that is dynamic like latest - 1 then you'll be able to set a fixed version in dependencies or if fixed versions are specified then you can safely rely on semver, but at least it would be great to have that specified explicitly like this (example from https://github.com/webcomponents/webcomponentsjs):

Polyfill IE11+ Chrome* Firefox* Safari 9+* Chrome Android* Mobile Safari*
Custom Elements
HTML Imports
Shady CSS/DOM

*Indicates the current version of the browser

@ArmorDarks
Copy link

Well, @idubinskiy is right that such breaking change shouldn't be released as patch....

@alexei
Copy link
Owner

alexei commented May 31, 2017

@idubinskiy if it broke on your live server, well good for you!

@ArmorDarks I thought semver refers to public API, not implementation details 😕

Re compatibility table: what do you guys use to test JS in various browsers? I mean how would one generate a compatibility table automatically?

@alexei alexei reopened this May 31, 2017
@ArmorDarks
Copy link

ArmorDarks commented May 31, 2017

@alexei It isn't just implementation detail. It's breaking change, which broke lib in old Node and IE9, thus compromised API for those consumers. I don't care much, because I nor use Node 0.12, nor target IE9, but I'm quite sure on some people it came unexpectedly, especially considering that it was bumped with patch, thus passed ^ selector.

| don't want to sound rude. Just want to eliminate misconception about semver.

Well, now there is not much can be done about it. You still can drop old version or revert changes and make proper releases chain, but I think most people already done something with it anyway.

@idubinskiy
Copy link

This is a somewhat relevant discussion on the SemVer repo: semver/semver#311

It's certainly not a settled matter. Just wanted to point that perhaps more care should have been taken. As far as I can tell, the README wasn't updated even with the known incompatibility for older browsers requiring a polyfill.

@alexei
Copy link
Owner

alexei commented May 31, 2017

I updated the readme file and I apologize for not doing so earlier.

@alexei alexei closed this as completed May 31, 2017
@alexei
Copy link
Owner

alexei commented May 31, 2017

Also I opened an issue to remind myself to set up some automated browsers tests. Any pointer would be appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants