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

Browser support policy #37

Closed
AndreasMadsen opened this issue Nov 18, 2013 · 20 comments
Closed

Browser support policy #37

AndreasMadsen opened this issue Nov 18, 2013 · 20 comments

Comments

@AndreasMadsen
Copy link
Collaborator

As a principal I believe that either you support a old browser or you don't, there should be no half solutions. It is way to complicated for those there use this project to care about browser support on a individual module or function level.

So since the big 2.0.0 bump we fully support IE8+. I made this decision because there was already some IE8 support and I didn't want more migration trouble for userland than they already where (caused by the node version bump).

Some time has now passed and pull-request #36 and issue #19 are good reasons to drop IE8 support. Since I'm quite distant from the browserify community I can't really make this decision.

/cc @alexgorbatchev @substack @Raynos @defunctzombie

@feross
Copy link
Contributor

feross commented Nov 18, 2013

@substack from IRC:

[06:52am] substack: feross: your thing should just do uint8array, don't worry about the fallback
[06:52am] feross: substack: okay, i can remove that
[06:53am] feross: substack: will you just do that check in node-browser-builtins?
[06:53am] substack: no it's probably best to just not check and push that choice onto the user
[06:53am] substack: no reason for everybody to suffer with a vastly larger bundle size
[06:54am] feross: okay, cool
[06:54am] feross: that's perfect

I'm pretty new to the browserify community so I don't know what people have come to expect. But I vote for dropping old browser support since life is too short to spend it supporting old software. browserify is for mad science!

@defunctzombie
Copy link
Contributor

If it can be supported with simple shims (via es5-shim or similar) then making the code free of browser nonsense is a win. The users that don't care about older browsers (more and more every day) will benefit from a smaller and more maintainable module and the module devs wont be burdened with debugging stupid browsers if they don't care about them. It is not hard to just add a note to the readme about "legacy" browsers. Really this comes down to the decision of the module authors. If you don't care about supporting old browsers (and you absolutely don't have to care) then just tell people to use a shim. There isn't a right or wrong here.

@ghost
Copy link

ghost commented Nov 18, 2013

We don't really need a universal support policy. In this particular case the downsides to old browser support (a much bigger bundle size) outweigh the benefits for this particular module. Other times it's rather easy to inline a little Array.isArray() function and everything works.

@AndreasMadsen
Copy link
Collaborator Author

Just to clarity, when I say I don't care its because I don't support IE8 in my own personal projects. However I'm still the one there gets the angry emails when things don't work and that I care about.

@defunctzombie
Copy link
Contributor

Tell them to use a shim. If they are writing an angry email to a volunteer
project maybe they should think about sending you some money first.
On Nov 18, 2013 10:46 AM, "Andreas Madsen" notifications@github.com wrote:

Just to clarity, when I say I don't care its because I don't support IE8
in my own personal projects. However I'm still the one there gets the angry
emails when things don't work and that I care about.


Reply to this email directly or view it on GitHubhttps://github.com//issues/37#issuecomment-28708639
.

@pkrumins
Copy link

What does this change mean to Testling? Testling browserifies test bundles and runs them in older browsers, such as IE6, 7, and 8.

@ghost
Copy link

ghost commented Nov 18, 2013

@pkrumins This will only affect modules that use the Buffer global. None of the modules in the testling stack should be affected.

@defunctzombie
Copy link
Contributor

We really need to get rid of this global use of browser and process. I
think it just takes a bit more convincing but the end result will be better
for us. It isn't like node doesn't introduce breaking changes, they just
pretend not to.
On Nov 18, 2013 12:04 PM, "James Halliday" notifications@github.com wrote:

@pkrumins https://github.com/pkrumins This will only affect modules
that use the Buffer global. None of the modules in the testling stack
should be affected.


Reply to this email directly or view it on GitHubhttps://github.com//issues/37#issuecomment-28716576
.

@pkrumins
Copy link

@substack great!

@alexgorbatchev
Copy link
Owner

I'm okay with supporting three latest versions of IE 9, 10 & 11. I would speculate that node stack is mostly used by startups and personal projects who don't often support IE8 and lesser. Those who need to support would be able to polyfill and get it work with a bit of extra effort.

I would love to have a paragraph in the readme talking about this.

Thoughts?

@AndreasMadsen
Copy link
Collaborator Author

Other times it's rather easy to inline a little Array.isArray() function and everything works.

Its actually a huge maintenance burden! @substack would it be possible to drop IE8 & 9 support entirely?

I'm okay with supporting three latest versions of IE 9, 10 & 11.

It will need to be IE10 & 11, since typed arrays isn't supported by IE9.

I would love to have a paragraph in the readme talking about this.

I'm thinking:

This module supports only IE 10+. This decision was made to reduce the maintenance and allow usage of more advanced browser features such as TypedArrays. Please do not supply pull requests there adds support for legacy browsers. Instead we recommend that you use the necessary polyfills for legacy browser support. Should you be willing to start your own complete browser-builtins polyfills project, then feel free to contact us for tips and hints.

@AndreasMadsen
Copy link
Collaborator Author

I've made a branch without legacy browser support so you can tell us how huge the impact is.

@AndreasMadsen
Copy link
Collaborator Author

Nobody have said anything and the new buffer constructor is almost done, so both #40 and #36 will be merged tomorrow. It will be a new major release so no worries :)

@alexgorbatchev
Copy link
Owner

Sounds great! Im in favor or dropping old IE support.

@AndreasMadsen
Copy link
Collaborator Author

@alexgorbatchev please publish version 3.0.0

By the way a single shims is needed in order to support IE11 :(

@defunctzombie
Copy link
Contributor

Do we have automated CI browser testing setup for this repo to avoid breakage?

@AndreasMadsen
Copy link
Collaborator Author

No and you are welcome to tell us how :)

PS: @alexgorbatchev is the one with admin rights.

@AndreasMadsen
Copy link
Collaborator Author

3.0.0 has been published

@alexgorbatchev
Copy link
Owner

I added travis, however it fails because of firefox...

https://travis-ci.org/alexgorbatchev/node-browser-builtins/builds/14818705#L444

firefox --no-remote -CreateProfile browser-launcher-62a8eb2 exited with code 1: Error: no display specified

@feross
Copy link
Contributor

feross commented Dec 2, 2013

What is being used to launch Firefox? Is that something that Travis or testling provides?

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

5 participants