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

add a build which includes polyfills for IE #85

Closed
XhmikosR opened this issue Aug 31, 2019 · 17 comments
Closed

add a build which includes polyfills for IE #85

XhmikosR opened this issue Aug 31, 2019 · 17 comments

Comments

@XhmikosR
Copy link

Since this is basically a development tool, I need to test things on IE11.

The current builds target explicitly only 3 browsers. That's way too strict IMO. It's just ~30% of the global market, see https://browserl.ist/?q=last+2+Chrome+versions%2C+last+2+Firefox+versions%2C+last+1+Safari+version%2C+not+dead

Please consider using a more lax config, like for example

>= 0.5%
last 2 major versions
not dead

https://browserl.ist/?q=%3E%3D+0.5%25%2C+last+2+major+versions%2C+not+dead

@XhmikosR
Copy link
Author

XhmikosR commented Aug 31, 2019

BTW this was introduced in 7d848a6#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R40 and there's no mention in the releases.

@bep
Copy link
Contributor

bep commented Aug 31, 2019

Note that Hugo currently uses #84 -- I haven't found the time to finish that PR (add tests).

@XhmikosR
Copy link
Author

Yup, I figured so from the Hugo git log. :)

That's the commit that introduced this issue because it's based on 3.x which they introduced this breaking change.

@smhg
Copy link
Contributor

smhg commented Aug 31, 2019

I guess it's fine to extend that list. It will impact build size and performance though.
So probably best to avoid browsers that aren't used for development.

@XhmikosR
Copy link
Author

Many people test things in multiple browsers and livereload is for development. I wouldn't care so much about performance compared to compatibility here, but it's your project and there's room for changes there.

IMO just adding IE11 will increase the size as much as it gets. The other browsers shouldn't really impact this a lot.

But anyway, let's agree that 30% is just too small :)

@smhg
Copy link
Contributor

smhg commented Aug 31, 2019

Not my project, just helping out :)

I'm definitely ok with IE 11 support. It only indeed more than doubles bundle size. You spend so much time in IE 11? You support other browsers too?

Don't focus on the 30%. That's quite meaningless as mobile support isn't very useful.

@XhmikosR
Copy link
Author

XhmikosR commented Aug 31, 2019 via email

@smhg
Copy link
Contributor

smhg commented Sep 1, 2019

Am I correct to assume you develop/test first in an evergreen browser? And only test IE once everything works in them?
In that case, do you get much value from livereload in IE? Or are the errors it now (likely) throws the most problematic part?
I'm just trying to get a better picture of your/other's needs.

@XhmikosR
Copy link
Author

XhmikosR commented Sep 2, 2019

It's not what I need, IMO. Livereload is a developer tool; it's not used in production. Performance isn't a big issue locally. I mean, most of us don't even minify CSS/JS in development.

As such, it should not have this kind of limitations, IMO. It's way too soon to target these browsers only.

Also, this wasn't mentioned anywhere in the releases nor in README.md. You did make it a major version bump, but still.

@smhg
Copy link
Contributor

smhg commented Sep 2, 2019

This is only about devs testing in IE (9-11). All other browsers are basically covered.
Regular use of IE is about 2%, but browserlist queries are meaningless in this context.

What I'm trying to do is figure out what the benefits are of supporting livereload on IE.
When they are meaningful, we definitely need to add support. If there are very few, it might not be worth it.

These are my current assumptions:

  • Probably close to no one develops in IE first.
  • Projects that have hard requirements on IE probably have automated tests.
  • When IE support is a nice-to-have, IE tests probably take place when things run fine in other browsers. In those cases it is likely about polyfilling one or more things (?).
  • Having livereload throw errors on IE is annoying. Do you get any?

Please help me out by filling in the gaps.

Edit: I've added a note about dropping IE support in the 3.0.0 release. Thanks!

@bep
Copy link
Contributor

bep commented Sep 2, 2019

My 50 cents:

  • This library's primary use case is for web development
  • If you develop a library that needs to work in IE, you need to do a visual inspection (in addition to any automated tests)
  • If you need to debug/fix IE specific issues, you really want LiveReload to help you out.
  • I'm pretty sure that most devs understands the above and don't mind some extra bytes from localhost even if they don't care about IE.

@smhg
Copy link
Contributor

smhg commented Sep 2, 2019

If you need to debug/fix IE specific issues, you really want LiveReload to help you out.

I guess that's mostly true in the case of CSS? JS differences with evergreen browsers are about including the right polyfills, no?

I'm pretty sure that most devs understands the above and don't mind some extra bytes from localhost even if they don't care about IE.

Well, that's hard to estimate. In the same sense as whether IE support is a necessity. Let's imagine all the extra IE polyfills would add a 100ms startup delay (random number). This library is used a lot. Altogether that would not be free.

But I guess even only for CSS debugging it's worth it. Maybe a separate build can be a solution.
Let's first make a release with #84 and then add this to 4.0.0.

@XhmikosR
Copy link
Author

XhmikosR commented Sep 2, 2019

Unless you have a benchmark, as it is now, we can assume there's no performance hit important enough to bother people who develop with this breakage.

If this wasn't a dev tool we wouldn't be discussing all this, of course.

@bep: we can also just maintain your fork since you already have a custom patch that's not merged.

@smhg
Copy link
Contributor

smhg commented Sep 2, 2019

Unless you have a benchmark, as it is now, we can assume there's no performance hit important enough to bother people who develop with this breakage.

Feel free to clarify why you assume adding +60KB of polyfills has no significant consequences.

If you mean compared to the burden of the IE 'breakage': in the last 5 months, since version 3.0.0, you are the first to report this. This tells me IE use might actually not be that common.

However, with a separate build, if feasible, we can tackle both. As always: PR's are much appreciated.

@smhg smhg changed the title Relax browserslist config add a build which includes polyfills for IE Sep 2, 2019
@AlexWayfer
Copy link

I've developing sites for IE11 too, and I test them in it sometimes, but I can live without livereload in this browser. But it can be helpful.

@XhmikosR
Copy link
Author

XhmikosR commented Sep 2, 2019

Again, without a benchmark you have no proof, nor do I. Even if I agree that it might be slower, without numbers, this is moot.

You could have at least mentioned this in the release notes or in README.md. It's quite a big assumption to make.

This issue is not about IE. It's about relaxing the browserslist config. People don't always live on the edge. You are even excluding Firefox ESR.

@smhg
Copy link
Contributor

smhg commented Sep 3, 2019

Just to make sure:
The browserlist config only affects the build output (dist/livereload.js), right?
Because I'm not aware of any other consequences it has. If there are, they would of course be important.

To summarize:
If there would be a build with all possible polyfills (e.g. dist/livereload-max.js), would it offer a (proper) solution?

Off-topic:

...and there's no mention in the releases.

Also, this wasn't mentioned anywhere in the releases nor in README.md.

You could have at least mentioned this in the release notes or in README.md.

Please keep issue reports to the point. If you think something can be done better, please contribute. Feel free to contact the owner of the project to take things in a different direction.

Backstory: with the 3.0.0 release, I completed a second attempt to convert the codebase from coffeescript to modern JS. This required a new build process. I chose babel based on a browserlist config I thought would suffice. There was no plan to drop support for anything, I just didn't image livereloading in IE was a thing. I've now added the breaking change in the release notes.

@smhg smhg closed this as completed in 223e63b Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants