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

Provide polyfills for browsers when necessary #1960

Open
datitisev opened this issue Jan 1, 2020 · 8 comments
Open

Provide polyfills for browsers when necessary #1960

datitisev opened this issue Jan 1, 2020 · 8 comments

Comments

@datitisev
Copy link
Member

@datitisev datitisev commented Jan 1, 2020

If we want to support IE 11, we are probably going to want to provide polyfills for the browser instead of manually adding them and changing our code to be compatible.

We could do this two ways:

  • generate a separate JS asset that includes polyfills from a package, e.g. core-js
    • + file is local
    • - will include polyfills that the browser may already have
    • - bundle size will be big
  • use an external service such as polyfill.io
    • + only adds the polyfills the browser needs
    • + we don't have to make a choice between bundle size & how many features the browser doesn't support
    • - file is hosted by a third party service

Having polyfills for at least IE 11 and probably a few other browsers would help both core development & extension developers.

However, the implementation is the tricky part here. Feedback welcome.

@tankerkiller125

This comment has been minimized.

Copy link
Contributor

@tankerkiller125 tankerkiller125 commented Jan 3, 2020

Although use of a local polyfill sounds great in terms of usability in environments that may not have access to CDN resources (restrictive firewalls, censored countries, etc) the network use increase from including unneeded polyfills would negatively affect both load times as well as mobile data usage for users with limited data plans on mobile.

The other downside of loading a polyfill locally is lack of browser cache, notably using a local polyfill, if I load 11 flarum sites each one is going to save the same file but as a different cache object meaning that every site will have downloaded the polyfill at least once (11 times total) which depending on the bundle size may also increase not only network usage but also disk space used.

Loading from a CDN as noted has a downside of being external, however it also comes with the fact that the bundle automatically generates with only the features needed for that browser meaning smaller network usage and because it's hosted on the same site not only for Flarum sites but also globally this means that the file only has to be cached by the browser once decreasing the use on disk and further reducing network usage. Using the example above it could be an 11x decrease in network/disk usage.

Something else to note about Polyfill.io is that it can be self-hosted if absolutely needed so an option to change the CDN path in the config or disable the polyfill altogether (companies that control which browser get used) might be a good option if pollyfill.io gets used.

@ShatteredMotion

This comment has been minimized.

Copy link

@ShatteredMotion ShatteredMotion commented Jan 6, 2020

The other downside of loading a polyfill locally is lack of browser cache, notably using a local polyfill, if I load 11 flarum sites each one is going to save the same file but as a different cache object [...]

Note that shared cache is being removed from major browsers ([1], [2]) due to privacy leaks.
So in your scenario the resource is going to be loaded 11 times regardless whether it was hosted locally or on some shared CDN. In fact, once the cache isolation is enabled by default in the future, hosting locally can become faster than using a CDN due to features in HTTP/2 and upcoming HTTP/3.

@tankerkiller125

This comment has been minimized.

Copy link
Contributor

@tankerkiller125 tankerkiller125 commented Jan 6, 2020

@ShatteredMotion I'll have to double check this but based on what I'm reading it appears that it's more blocking one site cache from accessing another one. But not preventing CDN resources from being cached the way they always have been.

Another note is that even if the partitioning works the way you say it does (which it might) polyfill.io still automatically detects the correct polyfills to load which will still significantly reduce network and disk use. Notable if I load the polyfill in IE I get 20kB of JS where as in chrome I get just 80bytes (just enough for the comment about what polyfill.io is)

@ShatteredMotion

This comment has been minimized.

Copy link

@ShatteredMotion ShatteredMotion commented Jan 6, 2020

[...] it appears that it's more blocking one site cache from accessing another one.

Which is exactly what happens when you load a file from a CDN which was previously loaded by another site, isn't it? The other site loaded the resource from the CDN into cache and now you try to read a file from cache that was not placed there from your site. This is exactly what is meant in the first point of the first link I have posted. You can monitor how long it took to load resource x from the CDN when a user visits your site and thus conclude whether the user previously visited another site that included the same resource. If the resource is only used on specific sites, you can thus conclude that a particular user visited a specific site before.

[...] polyfill.io still automatically detects the correct polyfills to load which will still significantly reduce network [...]

This is of course a big advantage of polyfill.io. But in case only specific polyfills are required by flarum (which I don't know, I have no idea about what IE11 can and can't do) and you only want to polyfill stuff for IE (instead of polyfilling brand new features for all major browsers for example), you could easily only dynamically load a polyfill file for IE11 and do nothing for other browsers. Then you would actually also save the whole connection to polyfill.io to load an essentially empty file in more modern browsers.

@tankerkiller125

This comment has been minimized.

Copy link
Contributor

@tankerkiller125 tankerkiller125 commented Jan 6, 2020

The problem I see with only loading polyfills for IE is that some people may be using older versions of Firefox, Chrome, Opera, etc. due to corporate policies or extended release versions. The other part is that this polyfill would also be potentially used by extension developers of which we have no control over. So if we only load polyfills required for Flarum core and core extensions they may use a feature that we didn't polyfill breaking their extension and potentially all of Flarum depending on what their extension does.

@franzliedke

This comment has been minimized.

Copy link
Member

@franzliedke franzliedke commented Jan 8, 2020

Let's talk about providing a local polyfill file for a moment: how would that work? I see two options really:

  1. Maintain the polyfill file in core. This would mean either

    • providing all kinds of polyfills, even when extensions don't need them, again increasing bundle size, or
    • only providing a select few, which means any others would again have to be bundled by extensions, which takes us back to the original problem
  2. Generate two bundles in core and extensions (one with modern JS, one with polyfills). This would have two problems:

    1. We cannot do bundling at runtime - in fact, none of the JS server-side tooling is run at build time. Therefore, Flarum core cannot e.g. generate polyfilled assets for extensions. Which brings us to...
    2. Backwards compatibility: old extensions wouldn't have such a bundle (and I really don't want to force them), and we don't have a good upgrade story for our webpack helper anyway. We'd have to assume the bundle to be polyfilled, and optionally use a "modern" bundle to compile a smaller build, if present.

I am a bit stumped here, which leads me to at least somewhat consider the remote option.

In any case, it's probably painful. So we should definitely avoid features that need to be polyfilled as much as possible, too. 😆

@datitisev

This comment has been minimized.

Copy link
Member Author

@datitisev datitisev commented Jan 8, 2020

@franzliedke The JS bundling would be done in core, producing two dist files with Webpack, one containing polyfills. It could also be done with external extensions I guess, looking at if a file with polyfills exists for the added JS dist file through the extender.

The problem with avoiding features that need to be polyfilled is that many of them are standard now. Things like Array.prototype.find have been used in many places - yet IE 11 does not support it. Same thing with Array.prototype.includes I think, which is supported by every major browser except IE.

I agree that there is not any great solution. Perhaps the best is simply to use the external service only on unknown browsers - we could have a list of minimum browser versions and check against it. If the browser is an older version that our minimum, we provide the polyfill. If the browser is not found in our list, we provide the polyfill. Thoughts?

@tankerkiller125

This comment has been minimized.

Copy link
Contributor

@tankerkiller125 tankerkiller125 commented Jan 9, 2020

@datitisev This is a solution I like a lot actually, providing the remote pollyfill on browser that don't meet the general minimum requirements and excluding modern browsers seems like a great idea as it saves networking/cache on modern browsers and at the same time ensures that everyone can enjoy Flarum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.