sveltejs / sapper

The next small thing in web development, powered by Svelte
https://sapper.svelte.dev
MIT License
6.99k stars 434 forks source link

Differential bundling test for modern browsers is inadequate #580

Closed arggh closed 4 years ago

arggh commented 5 years ago

Apparently the modern / legacy differential bundling feature fails to deliver the correct bundles for some browsers.

For example, spread syntax with object literals will cause some browsers to fail, such as Microsoft Edge, which doesn't support the syntax, but supports async await, which is Sapper's "litmus test" for a modern browser.

sapperbug2

Reproduction: https://github.com/arggh/sapper-syntax-issue

antony commented 4 years ago

Closing this because Edge doesn't need or use the legacy bundle, you just have to babel/buble/polyfill for object rest spread.

arggh commented 4 years ago

That polyfill doesn't get rid of the fact that Sapper's solution to differential bundling is suboptimal 🤷‍♂️

antony commented 4 years ago

@arggh actually the polyfill isn't required for Edge at all. There isn't a rest-spread polyfill as it's a language feature.

I'm confused about the differential bundling issue though now - your complaint relates to edge, which shouldn't use the legacy bundle, and it correctly detects IE, so which targeted browsers are not correctly detected by the test?

arggh commented 4 years ago

My point exactly. The Edge version you can see in my screenshot apparently supported async await, and therefore was shipped the modern bundle, which broke the app since the bundle contained other untranspiled syntax that the Edge version did not support.

What I'm trying to say is: using the async await -test as a black and white criteria for deciding between modern & legacy bundles is prone to fail and will not even be helpful in the future, since "modern" and "legacy" aren't set in stone. They evolve and change.

antony commented 4 years ago

@arggh as as I understand, the legacy bundle is designed only for browsers which are beyond help, namely - IE. Edge gets the modern bundle.

This isn't to say that everything works in Edge because that isn't true. Edge and its ilk will need a small amount of transpilation/polyfilling inside the modern bundle if you want to use those features.

The criteria has to be black and white because it's Modern or Legacy. There is no value in having a different bundle for each browser - that's what preset-env, polyfill.io, core-js and autoprefixer are for.

If you're using advanced features which break Edge then the onus is upon you to transpile/polyfill for them. As an example, we have a rather large site with a lot of third party dependencies where nearly no effort has been made to support anything below modern Chrome, Safari, and Firefox. We babel the legacy build and add some polyfills as per my project. I removed use of object rest spread and now the site works across all browsers flawlessly - that's where the sapper-ie project came from.

What I'm saying is that if you were to use some relatively rare feature unsupported by edge, then you would be expected to test and provide support for that yourself, via the modern bundle.

since "modern" and "legacy" aren't set in stone. They evolve and change.

all browsers and versions released for a few years now will work with the modern bundle, so it's set in stone. legacy browsers aren't receiving updates, so those too are set in stone.

I understand what you're saying but I'm (poorly) trying to explain that legacy and modern aren't there to help you support differences across various browsers. they're there as a safeguard so that you can support dead, obsolete browsers, sadly still in use in small amounts, which have been abandoned (set in stone). For everything else, there's transpilation.

If you had a concrete example (platform + version) of a browser being served the wrong bundle, then this is something that could probably be looked into, but I don't know of any?

arggh commented 4 years ago

Here is a good writeup on how Meteor implemented differential bundling: https://blog.meteor.com/meteor-1-7-and-the-evergreen-dream-a8c1270b0901

The krux (as I seet it) is this: both frameworks ship two bundles, legacy and modern. Given that the app uses spread syntax, only the app built with Meteor will actually work out of the box on the Edge browser used in the screenshot. The one built with Sapper will not work.

Therefore, Sapper's solution is inferior to Meteor's solution and can be improved to become equal or better than Meteor's solution.

antony commented 4 years ago

I'll have a read when I have a spare minute, but understand that the goal of Svelte/Sapper is to compile for modern browsers, and make backwards-compatibility available if required. Transpiling for old Edge out of the box was never the intention.

I do like what I can see of the meteor approach, and I will agree that Sapper's differential bundling isn't as nice as that, and maybe it would be a nice feature, so feel free to raise a new issue so we can discuss approaches for that, but the first line of this ticket is "differential bundling feature fails to deliver the correct bundles for some browsers" and that simply isn't true. It delivers the correct bundles for the correct browsers - which is why I believe it should be closed.

arggh commented 4 years ago

It delivers the correct bundles for the correct browsers

We have to agree to disagree!

  1. Spread syntax can be transpiled away by Babel.
  2. Sapper transpiles the legacy bundle with Babel.

    <--- room for improvement

  3. Sapper delivers the modern bundle for a browser that doesn't support spread syntax.