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

ie8 #23

Closed
TrySound opened this issue Sep 15, 2015 · 5 comments
Closed

ie8 #23

TrySound opened this issue Sep 15, 2015 · 5 comments

Comments

@TrySound
Copy link

Do we need ie8 support now? If not, you can remove 3 depenendencies.

@jhiesey
Copy link
Owner

jhiesey commented Sep 15, 2015

Given that other modules used by browserify are no longer supporting ie8 (see defunctzombie/node-url#6), I think this is inevitable at some point.

I see two possibilities:

  • Remove all code that can be replaced by global polyfills and tell the user to include the needed ones (like for Object.keys, String.prototype.substr, etc.), but retain code that can't be replaced by polyfills
  • Rip out all code needed for ie8 and make it completely unsupported

Let me ask on irc before making a decision

@jhiesey
Copy link
Owner

jhiesey commented Sep 15, 2015

Per discussion on irc, we'll keep ie8 support for now. But requiring polyfills will probably come at some point.

@jhiesey jhiesey closed this as completed Sep 15, 2015
@parshap
Copy link

parshap commented Sep 15, 2015

IRC log for context:

1:45:48 PM jhiesey> substack: what do you think about ie8 support in browerify?
1:46:16 PM jhiesey> I got this issue on stream-http, which browserify uses as the http shim: #23
1:55:27 PM substack> jhiesey: I think if the code is working fine there's no point is breaking it
1:55:58 PM jhiesey> it's working ok other than adding to the bundle size
1:56:13 PM jhiesey> but it's also getting increasingly difficult to maintain
1:58:23 PM jhiesey> substack: i did have to turn off the ie8 tests (even though i know it works) because i keep hitting the unresponsive script dialog in various places
2:00:19 PM jhiesey> most recently and unavoidably in the event loop turn that runs all of the require()s and initialization code, most of which time isn't spent in my code
2:01:08 PM parshap> jhiesey: i'd prefer to see ie8 support remain but would be ok with leaving it up to the end-user to include needed global polyfills
2:01:20 PM jhiesey> ok will keep it for now
2:02:06 PM parshap> jhiesey: even though we don't officially support IE8, most of our UI (including React) does, and it'd be nice to one day be able to officially support it
2:02:39 PM parshap> jhiesey: and our "your browser is not supported" page also attempts to make some http calls, so it'd be nice if that continues to work for ie8 users
2:02:54 PM parshap> (but, honestly, it's not critical)
2:03:01 PM jhiesey> parshap: that's a good point
2:04:48 PM jhiesey> parshap: it will be hard to keep support in the future though if modules like url refuse to do so
2:08:26 PM parshap> jhiesey: indeed, but that doesn't quite seem to be the case yet
2:09:08 PM parshap> jhiesey: i think it'd be fine (and maybe even preferable) to require the user to include needed polyfills for e.g., substr(-1)
2:21:24 PM jhiesey> parshap: the original issue was mostly about requiring the user to include polyfills for Object.keys, Array.prototype.forEach, and String.prototype.indexOf instead of bloating the bundle with required implementations. Do you think that would be reasonable? If so I can rip those out and document what's required
2:22:41 PM parshap> jhiesey: yes, i think that's be good so not everyone gets the polyfill bloat
2:22:56 PM parshap> jhiesey: for the most part, i think it's reasonable to assume an es5 environment
2:24:44 PM parshap> jhiesey: that said, there may be ie8 issues that are not fixable with a polfyill, and in that case it'd be nice if stream-http had fixes built-in
2:38:42 PM jhiesey> parshap: will do

@TrySound
Copy link
Author

@jhiesey We can start with the first possibility.

@jhiesey jhiesey reopened this Sep 15, 2015
@jhiesey
Copy link
Owner

jhiesey commented Sep 15, 2015

Per more discussion, let's do the first option

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

3 participants