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

How can I check mobx version in runtime #2038

Closed
Mess1ah opened this issue Jul 9, 2019 · 23 comments
Closed

How can I check mobx version in runtime #2038

Mess1ah opened this issue Jul 9, 2019 · 23 comments

Comments

@Mess1ah
Copy link

Mess1ah commented Jul 9, 2019

Welcome to MobX. Please provide as much relevant information as possible!

I have a:

  • [ x ] Question: I wonder how to get mobx version in runtime . I have noticed that It has already export MobXGlobals in globalstate.ts and it has a version property. But it writes in annotation:

this version is unrelated to the package version of MobX, and is only the version of the internal state storage of MobX, and can be the same across many different package versions

I want to check the mobx version in the project to avoid the problem causing by using both mobx 4 and mobx 5.

@danielkcz
Copy link
Contributor

danielkcz commented Jul 9, 2019

Probably duplicate of #1957
Please read through that and if you still have a question, elaborate on what you have in mind exactly.

@Mess1ah
Copy link
Author

Mess1ah commented Jul 10, 2019

Sorry, I don't think my question is duplicate of that one.

Probably duplicate of #1957
Please read through that and if you still have a question, elaborate on what you have in mind exactly.

My question is can mobx export the package version in global variable. Just like what React do in the picture below.
image

The situation is when some packages depend on mobx 4 and if user install mobx 5 , there will be 2 different mobx versions in project which will cause some problems. So I need check the mobx version in my package which depends on mobx 4 and throw out errors when I detect the mobx version installing in the project is mobx 5.

At present, Mobx has already export MobXGlobals in globalstate.ts and it has a version property. But it is not the package version so it can't solve my problems

@danielkcz
Copy link
Contributor

danielkcz commented Jul 10, 2019

Alright, that's certainly an interesting idea, but comes with overhead on who is going to be updating that version number :) Are you willing to investigate how React does that (I doubt it's a manual process)?

I am not entirely sure how the release process works here yet. If a regular npm version command is used to set a version or what.

ping @xaviergonz @ItamarShDev

@Mess1ah
Copy link
Author

Mess1ah commented Jul 10, 2019

I investigated React source and found out React has a file called ReactVersion. It looks like changing manually

@Mess1ah
Copy link
Author

Mess1ah commented Jul 11, 2019

Maybe we can define a property in export modules and replace by rollup-plugin-replace while building. The property value equals to version number in package.json. It will change version property automatically. But I noticed that the project changes version number when running publish script, but the publish script will run after build

@danielkcz
Copy link
Contributor

Build definitely runs before publish, we don't publish old version :) Rollup plugin might be not such a good idea because we want to eventually switch over to some build tool like microbundle.

@Mess1ah
Copy link
Author

Mess1ah commented Jul 11, 2019

Yep, of course we should build before publish. Maybe we can use prepublish hooks to change version in package.json and build the package?

@danielkcz
Copy link
Contributor

Man, I cannot give you a straight answer here because I am not doing publishing in this repo. Let's wait for someone responsible to answer that.

Either way, what I wanted to suggest. Isn't actually feature testing better than relying on a specific version? For example, when React Hooks came out, many libraries opted for checking if React.useState is available to verify a necessary version. With all those alpha versions it would be hard to check that properly.

I can't tell you what exactly V4 has and what V5 doesn't, but I am sure you can find some difference there and base your decision on that.

@danielkcz
Copy link
Contributor

Btw, your decision on version number would be also slightly crippled and relying on people to have the latest version from either branches. With feature testing, you can do right now even with the older versions.

@Mess1ah
Copy link
Author

Mess1ah commented Jul 11, 2019

Yep, your words make sense. I will figure out can I distinguish between V4 and V5 by checking api difference. Thanks for your idea.

@mweststrate
Copy link
Member

The situation is when some packages depend on mobx 4 and if user install mobx 5 , there will be 2 different mobx versions in project which will cause some problems.

This should be picked up and detected automatically by MobX already, and there should be no need for you to check this manually. (That is exactly what that version property is for btw).

If you have an actual problem with it, please provide a reproduction, otherwise this issue can be closed as you are trying to solve an issue that MobX should already take care of for you :)

@farwayer
Copy link
Contributor

This can be useful for third-party libraries that peerDepends on MobX.

@Mess1ah
Copy link
Author

Mess1ah commented Jul 15, 2019

I have checked out the error trace. It is caused by the following code:
image
This code is invoked by following code which is simply set a symbol type key in the object. And because of Object.proxy, It calls the code showing in the first photo.
image
When running the conditional sentence !isNaN(name)(name is a symbol type variable), It causes an error.
So I guess it maybe a bug? Certainly I can avoid it by using Mobx4, so in order to prevent this similar situation from causing errors, maybe checking version by user is essential.

@mweststrate
Copy link
Member

@Mess1ah it is completely unclear what "the error" refers to, or how this relates to the current issue atm.

Note btw that matching libraries is not a runtime problem, but a design time problem, and setting peerDependencies of your package correctly should result in your package manager giving warnings about this.

@danielkcz
Copy link
Contributor

danielkcz commented Jul 15, 2019

of your package correctly should result in your package manager giving warnings about this.

I don't know how about you, but half of those peer dependencies warnings are false-positive coming from a wrong understanding of either semver or the whole mechanics. I tend to ignore these warnings which isn't right, but the whole concept is wrong and kinda leading to it. All I am saying it's not a golden bullet.

@Mess1ah
Copy link
Author

Mess1ah commented Jul 15, 2019

Here is the example code:

const mobx = require('mobx');

let todos = mobx.observable([
    { title: "Spoil tea", completed: true },
    { title: "Make coffee", completed: false }
]);

// set some symbol-type keys on an array for some logics
let a = Symbol('a');

// next line will cause an error in mobx5 but won't in mobx4.
todos[a] = 'test';

These code will run good in Mobx 4 and get errors in Mobx 5. The error trace is

/yourProjectName/node_modules/mobx/lib/mobx.js:2943
        if (!isNaN(name)) {
             ^

TypeError: Cannot convert a Symbol value to a number
    at isNaN (<anonymous>)
    at Object.set (/yourProjectName/mobx-error-demo/node_modules/mobx/lib/mobx.js:2943:14)
    at Object.<anonymous> (/yourProjectName/index.js:12:10)
    at Module._compile (module.js:653:30)
    at Object.Module._extensions..js (module.js:664:10)
    at Module.load (module.js:566:32)
    at tryModuleLoad (module.js:506:12)
    at Function.Module._load (module.js:498:3)
    at Function.Module.runMain (module.js:694:10)
    at startup (bootstrap_node.js:204:16)

It seems a little bit strange but for some reasons I need set some symbol typed value on array and It is allowed in javascript.

It can be avoided by using Mobx4, so in order to prevent this similar situation from causing errors, I open this issue to ask whether there is a way to get the mobx version 😁

@danielkcz
Copy link
Contributor

danielkcz commented Jul 15, 2019

Hmm, could that be a bug? It should probably recognize a Symbol before falling back to thinking it's a number. However, you shouldn't be accessing elements of array with Symbol. You don't do that with regular JS either, do you? If anything, it should an object imo. It's a really weird use case.

@mweststrate
Copy link
Member

mweststrate commented Jul 15, 2019 via email

@danielkcz
Copy link
Contributor

@Mess1ah I wonder. Is this whole issue happening because one user of your lib has run into this particular problem? You should have probably started with that instead of trying to figure version detection :D

@Mess1ah
Copy link
Author

Mess1ah commented Jul 15, 2019

@FredyC This is not the case, I know exactly how to use mobx ~ I add these keys to tag the variable for some extra logics and these logics are not about value observable. And In Mobx 4 it goes well😁

@danielkcz
Copy link
Contributor

Yea I suppose it would make sense, in that case, to add a condition when Symbol is used, then just set it on that object. Can you please open a new issue with details and reproduction on this particular case? It's getting fairly convoluted in here.

@Mess1ah
Copy link
Author

Mess1ah commented Jul 15, 2019

Okay,I have opened a new issues about it 2044. Thanks for your time~

@Mess1ah Mess1ah closed this as completed Jul 15, 2019
@lock
Copy link

lock bot commented Sep 13, 2019

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.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants