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

Drop IE8 support #46

Open
dominykas opened this issue Sep 5, 2014 · 12 comments
Open

Drop IE8 support #46

dominykas opened this issue Sep 5, 2014 · 12 comments
Milestone

Comments

@dominykas
Copy link
Contributor

I know, I know, it was just added :) But still - I'm raising this issue to at least get an official "no" - or maybe someone has a more concrete idea on how to decouple IE8 support / inject the header parsing function?

IE8 support depends on parse-headers, which is an extra 1.5kb browserified+minified. I'm using xhr itself exactly because it is small - ~2kb browserified+minified, so IE8 support effectively doubles the size. Some might say that's not much, but over here we have to save every byte (building a script for 3rd party sites...). Plus IE8 is not a Real Browser™ anyways.

@naugtur
Copy link
Owner

naugtur commented Sep 5, 2014

I understand your concerns.

But the version with parse-header is also a better replacement for request module in node, so an app can use the same code to work in both node and browser (with just the module switch).

Creating a lightweight version of xhr would solve the problem.

Assuming you're using browserify, as a workaround, you can replace parse-headers with a noop function (see docs for package.json browser field https://gist.github.com/defunctzombie/4339901) - your browserify build would then create a version of xhr that returns undefined in headers field and is back to the original size.

@Raynos
Copy link
Collaborator

Raynos commented Sep 5, 2014

I wouldnt mind moving IE8 support to a seperate module maybe require('xhr') and require('xhr/ie')

@naugtur
Copy link
Owner

naugtur commented Sep 5, 2014

xhr/ie looks good. We'd have to make those two share some code responsible for the API to prevent them from losing sync. But I'd like that split.

I got to use xhr for the same reason (of small size) in the first place.

@pluma
Copy link

pluma commented Dec 17, 2014

-1 for dropping IE8 support entirely. I'm currently using xhr in a project that will have to support IE8 for at least another year, maybe longer. I'm also using xhr in arangodbjs, which works fine with IE8 otherwise. Also, IMO xhr's main selling point is being a browserify substitute for request.

+1 for xhr/ie

@davidtheclark
Copy link

I'm interested in this split, too.

@naugtur I've tried to use the browser field to remove parse-headers from browserify code, as you suggested; but it doesn't seem to work as expected. I am using lodash-node throughout so I tried (in package.json):

"browser": {
    "parse-headers": "lodash-node/modern/utility/identity"
  }

But I still see the parse-headers code in my compiled JS.

I understand that this is not the ideal venue to address a question like this --- but I thought that since you brought up the solution here, maybe you would have a quick response that would be useful to others who run across this issue and try that solution. If not, I'll look elsewhere to figure out what I'm doing wrong. Thanks.

@naugtur
Copy link
Owner

naugtur commented Feb 14, 2015

It should work. I'd need to get a copy of your code to be able to help.
Post the question on stackoverflow with parts of your package.json and a link to repo if it's public, then mail me the link. cya there.
We'll clean up this thread after that.

@bevacqua
Copy link
Contributor

It won't work. Browserify considers the browser field on a package per package basis, not globally.

@naugtur naugtur added this to the v3.0 milestone Oct 5, 2016
@jimmywarting
Copy link

jimmywarting commented Mar 15, 2017

It should be removed by now. Microsoft only give out support for ie 11

@naugtur
Copy link
Owner

naugtur commented Mar 15, 2017

Don't worry, we are aware of what's happening in the browser world.

Did you mean to say anything other than letting us know?

@Raynos
Copy link
Collaborator

Raynos commented Mar 15, 2017

10/10 issue management.

would give ownership to @naugtur again.

@dominykas
Copy link
Contributor Author

stalebot-ing myself.

@naugtur
Copy link
Owner

naugtur commented Apr 13, 2018

The code is on a branch. It'll get in as soon as other breaking changes for next major are implemented, but this got put on hold a few months ago as my daughter was born and other contributors weren't too active either. I'll get back to it eventually.

Feel free to take a look at #81 to help push it forward

@naugtur naugtur reopened this Apr 13, 2018
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

7 participants