Skip to content
This repository has been archived by the owner. It is now read-only.

Allow a switch to disable building of the legacy bundle #333

Closed
KoenLav opened this issue Oct 10, 2018 · 25 comments
Closed

Allow a switch to disable building of the legacy bundle #333

KoenLav opened this issue Oct 10, 2018 · 25 comments

Comments

@KoenLav
Copy link

@KoenLav KoenLav commented Oct 10, 2018

If the project needs to support legacy browsers (which is already a subset of projects) there are only two scenarios in which this work would be useful:

  1. If some kind of compilation error would cause the legacy build to not be successful;
  2. If some kind of compilation quirk would render different functional results between legacy and modern.

In short what we have to consider is; is it worth to impose the burden of double compilation on all developers, for the eventuality of some edge cases occurring in legacy compilation, whereas even when these edge cases occur they are only relevant for a subset of projects.

In my opinion the answer to that question, by default, should be no.

I can however imagine with different project experience you might answer yes to this question, but in my opinion at least the option to disable this should be provided.

@benjamn
Copy link
Member

@benjamn benjamn commented Oct 10, 2018

Would you be satisfied with a command-line flag like meteor run --no-legacy?

@KoenLav
Copy link
Author

@KoenLav KoenLav commented Oct 10, 2018

Definitely!

=====

Although I do still believe that it is a bit 'backwards' to 'burden' all developers with this additional cost.

Let's say 50% of projects require support for these legacy browsers. In the unlikely event of the legacy bundle producing an error during the build process, 50% of the people will care, whereas another 50% would not.

Out of those 50%, let's say 10% locally test their code on old browsers for every change. In the even more unlikely event that the legacy bundle produces code which behaves differently from the modern bundle, only 5% of total Meteor users would get a chance to notice this.

It seems more reasonable to me to accommodate these users with a flag to enable legacy builds, rather than to ask all the others to disable it. Actually, when thinking about it, I think you already do this right now using the --production switch (which performs production bundling, rather than development bundling).

I think we can assume all developers make use of a modern browser, so it doesn't seem logical to bundle legacy code for a development build (the --production flag indicates that a meteor run, without --production, creates a development build). When users want to pick up errors for legacy builds before releasing, they should probably configure their CI or local tests to use the --production flag.

Hence I really do believe the default behavior should be to not build the legacy bundle, but I'm more than happy to configure our development scripts to apply a --no-legacy option!

@merlinpatt
Copy link

@merlinpatt merlinpatt commented Oct 11, 2018

While I do agree that the majority of cases do not need to support legacy browsers, that policy does not fit in with Meteor's level of backwards compatibility. The only way the default will switch to only supporting modern browsers is if / when Meteor switches to 2.0 and can declare breaking API changes. Until then --no-legacy is the best we can ask for.

When --no-legacy is implemented, it should also be added to other options like meteor build --no-legacy and any other commands that would use it.

@sebakerckhof
Copy link

@sebakerckhof sebakerckhof commented Oct 11, 2018

@KoenLav Next to backward compatibility now, it's also important for backwards compatibility in the future. Because legacy is a moving target (currently roughly browsers that support async/await). And because not all browsers are auto-updating, you may want to support legacy browsers some day in the future (e.g. a 2 year old safari browser). So in order not to break when the definition of legacy shifts due to some ES20xx features, it's better to have it enabled by default. It's easy enough to make an alias / npm script / whatever that inlcudes --no-legacy

@coagmano
Copy link

@coagmano coagmano commented Oct 11, 2018

Another idea is to control it via a package like insecure and autopublish.
That way it's included by default and you can remove legacy entirely by removing the package?

That would be useful for folks that don't need legacy at all.
After all, with the delayed building in 1.8, the cost of the legacy bundle in development is now negligible

@sebakerckhof
Copy link

@sebakerckhof sebakerckhof commented Oct 11, 2018

@coagmano there's already a file for that (.meteor/platforms) where browser could be split in legacy and modern. But I think a very common use case is to develop constantly on modern browsers and only once in a while check a legacy one. So the --no-legacy option seems to be the best to me.

@coagmano
Copy link

@coagmano coagmano commented Oct 11, 2018

@sebakerckhof Just tried it out and no combination of browser, browser.legacy, or browser.modern (all with and without web. prefix) made any difference.
I also tried using various combinations of meteor add-platform and meteor remove-platform to no effect.
So I don't think what you describe works at the moment

Though using the platform commands does make more sense than a package for totally disabling a legacy bundle

I do think that --no-legacy is simpler, but now that we have delayed builds, when would this be useful?

@KoenLav
Copy link
Author

@KoenLav KoenLav commented Oct 12, 2018

@sebakerckhof

"But I think a very common use case is to develop constantly on modern browsers and only once in a while check a legacy one. So the --no-legacy option seems to be the best to me."

This vouches for only building the legacy bundle when using the production switch, actually.

"And because not all browsers are auto-updating, you may want to support legacy browsers some day in the future (e.g. a 2 year old safari browser)."

Good point, but still, this feels backwards. Because some vendors decide to not move ahead their installed base we are held to support browsers which we can pretty much consider to be broken?

I think the responsibility in that regard has already switched to the vendor, rather than the developer.

In addition to that this still vouches for only building the legacy bundle when building for production, and displaying a warning when a legacy browser is used in development, as we can expect developers to move ahead with time.

@KoenLav
Copy link
Author

@KoenLav KoenLav commented Oct 12, 2018

@coagmano

While delaying the legacy build limits the effect of the issue, it doesn't mitigate it completely; because a core will still be maxed out while making changes to the code, and this causes the server process to take more time to respond to new changes.

Even when we would make the legacy build cancellable (while we're at it we might as well make both builds cancellable), it still has some impact.

As we don't need this impact, I'd like to mitigate it all together.

@benjamn I think a switch would be a nice starting point, but I would definitely like to ask you to consider only building legacy when the production switch is applied, as I think only a handful of developers will check the legacy bundle (ever, when they are not debugging for it specifically).

@sebakerckhof
Copy link

@sebakerckhof sebakerckhof commented Oct 12, 2018

@coagmano . I didn't mean it works like that today. I meant there's already a place to say which platforms you want to target (e.g. meteor add-platform ios saved in the .meteor/platforms file), so to achieve what you propose that would be a more appropriate way than with a package.
It's true that the delayed build makes things better, but they still use CPU and can in some edge cases block the server (meteor/meteor#10261) or at least delay it on some CPU's.

@KoenLav

This vouches for only building the legacy bundle when using the production switch, actually.

Yeah, but if I then need to debug a problem with the legacy bundle, I each time have to wait for the extremely slow minification and so on to take place during development and I think I loose sourcemaps as well. I could disable that. But that's more work than adding --no-legacy to some alias.

Furthermore, while Meteor is zero-config, there's still a lot of new concepts to learn for new users. To make onboarding as easy as possible I think it should support as many browses as it can out of the box. This optimization of rebuild speeds will only make sense once your applicaiton has grown and by then you may have learnt about the --no-legacy flag. I don't think we should expect new users to also know about a --legacy flag when they need to support IE11 or so.

Good point, but still, this feels backwards. Because some vendors decide to not move ahead their installed base we are held to support browsers which we can pretty much consider to be broken?
I think the responsibility in that regard has already switched to the vendor, rather than the developer.

True, but unfortunately your users won't see it that way. For them it's your application that's broken. And for a lot of us these decisions are not made by the developers but by some product manager. Saying "It's the browsers fault" is an answer my customers generally don't accept. And on iOS there's not even an alternative to safari... (since chrome is also just safari)

@KoenLav
Copy link
Author

@KoenLav KoenLav commented Oct 12, 2018

@sebakerckhof --no-legacy it is!

@dovydaskukalis
Copy link

@dovydaskukalis dovydaskukalis commented Oct 12, 2018

--no-legacy would be perfect. It's a shame that right now half of the rebuild time is being spent for legacy browsers which we don't support in our project.

@pmcochrane
Copy link

@pmcochrane pmcochrane commented Oct 12, 2018

--no-legacy would be a preferred option for myself. If it was simple enough to achieve then adding options to disable all forms of builds would probably be useful for testing in specific environments

e.g
--no-legacy
--no-modern
--no-server (I don't need this option at all but including it for completeness)

I'd also include the builds for IOS and android but I don't know what they are called as I do not use them.

I can just about see me using a --no-modern option for when I was specifically targeting testing in a legacy browser.

@sabativi
Copy link

@sabativi sabativi commented Oct 15, 2018

Before this can be done, could qualialabs package do the job ?

@pmcochrane
Copy link

@pmcochrane pmcochrane commented Oct 22, 2018

@sabativi I had a look at the qualialobs:one package today and it does do what I need it to do. Thanks very much for the pointer as I have not come across any mention of this package before.

For people who have not tried it, this gives control of if you only want to compile the modern or the legacy package via an environment variable. It does what I need meteor to do.

The only drawback to the package is that it needs to be git cloned to a local package for it to work. I'm not sure if it would need to be updated when meteor updates (possibly not as it is quite small).

If there was some way to make this package into a meteor pull request with command line options instead of env variables it would be most appreciated by myself. Up until then, I'll continue to use the qualialabs package.

@marcuscarr
Copy link

@marcuscarr marcuscarr commented Feb 21, 2019

Another use case for this: meteor test appears to build both the modern and the legacy browsers. We only run server-side tests, so neither bundle is needed but both are built.

@ephemer
Copy link

@ephemer ephemer commented Mar 25, 2019

Any update on this? None of our projects support legacy browsers anyway, would be awesome to remove this overhead.

@RobertLowe
Copy link

@RobertLowe RobertLowe commented May 30, 2019

Also pinging, this would be nice to cut away from.

@jamesmillerburgess
Copy link

@jamesmillerburgess jamesmillerburgess commented Jun 19, 2019

We would also like this, but even further - could we disable both client bundles as well?

We have several people focused on back-end and Apollo schemas, and frequently they just need the server process running, so they can test via the playground.

@rcsandell
Copy link

@rcsandell rcsandell commented Aug 13, 2019

This is related to the issue being discussed, that sometimes you want to make sure you have legacy compatibility.

A while ago I had an issue with the legacy browser requirements changing, and Chrome 63 on the iPad mini no longer loading meteor apps. (it started randomly, was fixed randomly, and at some point again randomly started breaking again).

meteor/meteor#10395

I had to re-implement the fix linked in the issue above. To be honest if the legacy/modern browser compatibility bundles is going to randomly break older browsers, then I'd rather only use the legacy build, because who knows which older browsers, and when, they'll be broken.

sebakerckhof added a commit to sebakerckhof/meteor that referenced this issue Dec 18, 2019
This commit implements an --exclude-archs arg to the run command
which allows you to pass a comma-seperated list of architectures
to exclude.

Motivation:
Improve rebuild speeds, by allowing to leave out certain
web architectures during development.

Meteor rebuild speeds, although greatly improved in recent versions,
is still a common point of frustration as seen in the issues (meteor#10800)
and on the forum (https://forums.meteor.com/t/2x-to-3x-longer-build-time-also-on-testing-since-1-8-2-update/51028/3)

One reason for slow rebuilds is high memory pressure (meteor#9568).
The web.browser.legacy architecture is currently always being build
while most of us will only test this every once in a while.
So while we don't need it most of times during development
it does add to the high memory usage.
Furthermore, even though these builds are delayed,
long-running synchronous tasks have the potential to block the server startup (meteor#10261).
This last issue can be partially, but not fully, improved by meteor#10427
Therefore a commonly requested feature has been to disable the legacy build (meteor/meteor-feature-requests#333)

However, we may also temporarily disable cordova builds
and we may even want to disable just the web.browser build if we're just debugging
a bug in the legacy build.
Therefore I chose to create the more generic --exclude-archs option.
@gotjoshua
Copy link

@gotjoshua gotjoshua commented Dec 18, 2019

Would you be satisfied with a command-line flag like meteor run --no-legacy?

I don't understand why this isn't offered yet. It is scary to imagine trying to calculate how many hours of waiting happen every day and how many useless CPU cycles wasted, if we summed up all of us developers and all our changes...

@benjamn is this planned yet, or did you not manage to find a conceptual solution that you are comfortable with?

@StorytellerCZ
Copy link

@StorytellerCZ StorytellerCZ commented Dec 20, 2019

@gotjoshua There is a PR for this now: meteor/meteor#10824

@crapthings
Copy link

@crapthings crapthings commented Dec 20, 2019

awesome~

benjamn pushed a commit to sebakerckhof/meteor that referenced this issue Jan 13, 2020
This commit implements an --exclude-archs arg to the run command
which allows you to pass a comma-seperated list of architectures
to exclude.

Motivation:
Improve rebuild speeds, by allowing to leave out certain
web architectures during development.

Meteor rebuild speeds, although greatly improved in recent versions,
is still a common point of frustration as seen in the issues (meteor#10800)
and on the forum (https://forums.meteor.com/t/2x-to-3x-longer-build-time-also-on-testing-since-1-8-2-update/51028/3)

One reason for slow rebuilds is high memory pressure (meteor#9568).
The web.browser.legacy architecture is currently always being build
while most of us will only test this every once in a while.
So while we don't need it most of times during development
it does add to the high memory usage.
Furthermore, even though these builds are delayed,
long-running synchronous tasks have the potential to block the server startup (meteor#10261).
This last issue can be partially, but not fully, improved by meteor#10427
Therefore a commonly requested feature has been to disable the legacy build (meteor/meteor-feature-requests#333)

However, we may also temporarily disable cordova builds
and we may even want to disable just the web.browser build if we're just debugging
a bug in the legacy build.
Therefore I chose to create the more generic --exclude-archs option.
@cstrat
Copy link

@cstrat cstrat commented Jan 20, 2021

Shouldn't this be closed now, we have: meteor run --exclude-archs "web.browser.legacy"

Cheers for implementing this. I came across this issue after googling how to do this.

@filipenevola
Copy link
Member

@filipenevola filipenevola commented Jan 20, 2021

I think so.

I believe there is another discussion about enabling it for the build command as well but that should be a different feature request I believe.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests