-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[Proposal] MobX v6: use MobX v5 or v4 depending on the browser #1957
Comments
Proxy is not the only difference between v4 and v5, There were API changes (spy e.g.). |
Note that tree shaking features doesn't work in practice that well. Beyond that, sounds like a cool idea, if somebody is willing to experiment, go ahead :) Could be a great way to introduce MobX 6 which deprecates both 4 and 5 |
This maynot be a very hard features.The question is what should we do to fix the api changes? class App extends Component {
render() {
return (
<Shell>
{() => {
if (!window.Proxy || !window.Symbol) return import('./ProviderUseMobx4').then(({ default: Comp }) => <Comp />)
return import('./Provider').then(({ default: Comp }) => <Comp />)
}}
</Shell>
)
}
} |
How many breaking changes are there? If it's not too many between 4/5/6, we can monkey patch or create some Adapters. |
I am thinking if it's a really good course of action right now. Currently, we are maintaining version 4 as the LTS and backporting some bugfixes in there from V5. If we introduce V6 on top of that, it can become quite a maintenance hell to make sure everything is right across all three versions. I don't know, I don't see that big benefit there. We are talking mostly about supporting IE11 only which is getting older and older every day and chances are we would be able to abandon it completely one day. |
@FredyC My original idea wasn't to force maintainers to maintain a v4, v5, and v6, but to keep the v4 code together with the v5 code, which I believe would make maintaining it easier rather than more difficult, as "common ground" between the two versions could be refactored (if needed): possibly eliminating the need to backport bug fixes, as you put it. This doesn't even have to be a "v6", it could still be a v5 release with a deprecation of v4, since possibly no breaking changes would occur (this is something the maintainers would know better than me). The idea was more about allowing to load v5 or v4 depending on the browser (and giving that responsibility to MobX instead of to the users). You're right about IE11, of course, and abandoning it in the future would still be possible. Unfortunately, as much as I hate to say it, that doesn't mean I can simply not support it right now. |
@nunocastromartins I see your point, but you can hardly force everyone to upgrade to V6 right away and forget about 4 and 5 they exist and stop maintaining those. I do agree with you that it would be an ideal scenario, but not possible right now imo. If we would somehow "merge" V4 to be part of V5, it might be much harder in the future to separate it when it's relevant. Besides, if it would be that easy to support both approaches in a single version, I am sure it would have been done in a first place. But internal changes between those versions are fairly big from my understanding. Additionally, there are projects which can afford to ditch IE11 right now and it doesn't feel right to force upon them to include code they don't really need.
The core idea is certainly a good one, but I don't see any worthwhile way to achieve that. Ultimately it could be much easier to devise a good guide, possibly supply some tools for users to make that logic more approachable. |
@FredyC I understand where you're coming from, but isn't that how projects usually evolve? Say that you release v6 with no breaking changes for v5 users and a small set of breaking changes for v4 users (who, after fixing their code, would be using v5 in newer browsers automatically). I'm sure that not all v4 users could upgrade to v6 right away due to the breaking changes, but they can still use v4 in its deprecated state, knowing that v6 is a better alternative in the future. V5 users, however, could probably upgrade to v6 right away knowing that there would be no breaking changes (so it wouldn't differ much from your regular minor version upgrade).
I don't know MobX's code base well enough to understand how much of this is a problem, nor the reasoning for keeping two separate branches with different implementations, so I believe you if you think this is an actual problem. However "it might be much harder in the future to separate it when it's relevant" seems like the kind of issue that is usually not an issue if you tackle the refactoring process with this very concern in mind. But then again, outsider here, you might be right.
I agree with this, and this problem is what I was attempting to tackle in my first post, where I proposed different ways of importing MobX which could allow tree-shaking of unwanted code. Other solutions could possibly be provided.
That's also acceptable, it would at least be better than the current situation. Of course, as a user of the library, I'd prefer it if the library solved this issue for me. In any case, the reason I originally posted this proposal was that I believed (and still somewhat do) that it would make maintaining the library easier by having all code together in a single release version. It would also make MobX more "standards compliant" in a sense, since the way MobX is being released right now is a bit unique, and tools like those used to find outdated packages often complain about MobX being in v4 whilst a v5 has been released, for example. |
I don't know it either, I never actually contributed to the code of this repo, I am more about React related stuff. It's a mere assumption that if it would be that easy, it could have been done right away.
Would be great if you can put together some pointers what we need to explain in such guide, possibly supply some stubs for code utilities. Honestly, I am one of those who don't care about IE11, so I never had to deal with that issue. |
The implementation of reactive for objects, maps and arrays is entirely different between 4 and 5, while the API surfice is, intentionally 95% the same, and we backport features whenever we can. If your goal is to serve different versions to different users, there is a simpler solution: Creating two dummy packages, That being said, I don't entirely get the value of such an approach. If you have some users that cant use proxies, you are limited to the mobx4 feature set anyway, so there is not that much value in package mobx5 to those that can use it. You are just doubling your testing effort needed. |
Well, with the snippet from @cpp1992 it's not that hard to setup conditional lazy loading of different module based on browser capabilities. Unfortunately, it's not something that can be made into some shared utils package because it needs to be parsed by bundler (and node_modules isn't by default) to support that lazy load. That said, we don't really have benchmarks to compare V4 and V5 (or I am not aware of it). Unless there is some measurable difference, it's most likely not that bad to be using MobX 4 if you really must. It's surely an unfair thing. You might have like 2% of users who uses IE11 and you need to support them and because of that, the remaining 98% has to suffer from older MobX version. It's a business decision if that 2% is worth it or not. On our team, we have decided it's not and it's so easier to do things without such constraint (not only regarding MobX). I would kinda dare to say that as long as there will be software support IE11, it won't die that soon. Btw: Windows 7 won't be supported by Q1 2020 anymore and Windows 10 have the Edge by default. You can still run IE11 there, but such a person would be crazy. Perhaps it's time to reconsider business strategies :) Just saying. |
I suppose this is obsolete now considering that MobX 6 #2327 is in progress and there won't be two separate versions anymore. |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or questions. |
1 similar comment
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or questions. |
My use-case is the following: I need to support IE. However, I see considerable performance improvements with MobX v5 over v4, so I'd like to use v5 where possible and fall back to v4 when necessary.
I'm aware that this is possible to do on my side, but what is the reasoning behind not doing it within MobX itself?
I would suggest one of these two options:
Option 1
'mobx'
would use MobX v5 or v4 depending on the browser.'mobx/proxy'
when the user knows for sure that they don't need to support IE: this would allow a tree-shaker to shake away v4 code in production builds.'mobx'
could set an "internal flag" stating that the user is running MobX in "hybrid mode". When that flag is set, using MobX functionality that only works in MobX 5 (like adding properties to observable objects) should send a warning.Option 2
'mobx'
would use MobX v5, so by default the v4 code would be removed from production builds by a tree-shaker.'mobx/hybrid'
would use MobX v5 or v4 depending on the browser. The "hybrid mode internal flag" would be set here.What are your thoughts? If this were to be implemented, would you prefer something similar to the first or second options?
I feel like something like this is preferable to having two different branches within the same repository, from which two different versions of the same package are published.
The text was updated successfully, but these errors were encountered: