Package.json version management

bramis commented 1 year ago

It's been more than a year I'm using React-Admin Library for multiple projects and I really like how this library helping me building Admin and Dashboard interfaces 🎉 😍

But, I often faced on react-admin dependencies version management issues. It appear that, without even changing react-admin version (fixed version in my package.json file) and executing from scratch install (yarn install) on my project (without a yarn.lock file or node_modules folder), result in errors if one or more new versions of dependencies had been released and has changes that are in conflict with my current react-admin version.

This is mostly due to allowing automatic updates to the latest minor version for react-admin dependencies (^x.x.x).

Last recent example is with ra-ui-materialui where, without updating react-admin version in my package.json, I've done a fresh install of my project and the compilation failed because ra-ui-materialui has a new minor version, 4.11.0, and now Typescript throw errors related to the render prop of a FunctionField I'm using.

Another significant example happened few months ago with react-router-dom where for a previous react-admin version, a new minor version of react-router[-dom] has been released (6.10.x) and has caused failure on my project. Here again, I didn't change the react-admin version on my side and I was stuck with this error few days (because I wasn't aware that the react-router[-dom] has been updated because nothing changed in my package.json file and the error was not clear but it's another problem)

This also causing issue during my deployment process because a from scratch install is done on my side.

What you were expecting

Having my React-Admin project not breaking when I'm doing a from scratch install of my project, when I'm not touching my react-admin version which is fixed in my package.json.

What happened instead

Doing a from scratch install of my project resulting some times by a project broken because dependencies had a new minor version released

Steps to reproduce and Related Code

The use of ^ for packages version in the package.json file of react-admin :

"dependencies": {
        "@emotion/react": "^11.4.1",
        "@emotion/styled": "^11.3.0",
        "@mui/icons-material": "^5.0.1",
        "@mui/material": "^5.0.2",
        "history": "^5.1.0",
        "ra-core": "^4.10.2",
        "ra-i18n-polyglot": "^4.10.2",
        "ra-language-english": "^4.10.2",
        "ra-ui-materialui": "^4.10.2",
        "react-hook-form": "^7.40.0",
        "react-router": "^6.1.0",
        "react-router-dom": "^6.1.0"
    },

Here is my current package.json to avoid having surprises :

"resolutions": {
    "@emotion/react": "11.10.6",
    "@emotion/styled": "11.10.6",
    "@mui/icons-material": "5.11.11",
    "@mui/material": "5.12.0",
    "@mui/styles": "5.12.0",
    "@mui/utils": "5.12.0",
    "react-router": "6.9.0",
    "react-router-dom": "6.9.0",
    "ra-core": "4.10.2",
    "ra-ui-materialui": "4.10.2"
  },
  "dependencies": {
    "@emotion/react": "11.10.6",
    "@emotion/styled": "11.10.6",
    "@mui/icons-material": "5.11.11",
    "@mui/material": "5.12.0",
    "@mui/styles": "5.12.0",
    "ra-data-fakerest": "4.8.3",
    "ra-data-simple-rest": "4.8.3",
    "react": "18.2.0",
    "react-admin": "4.10.2",
    "react-dom": "18.2.0",
    "react-query": "3.39.3",
    "react-router-dom": "6.9.0",
    "react-scripts": "5.0.1",
    "typescript": "4.7.4",
    "web-vitals": "2.1.0"
  },

Other information

Automatic minor update shouldn't be allowed in your project regarding the dependencies you have, I would suggest at least allowing automatic patch version (~) or a fixed version in this case.

Btw, I love the project and what you React-Admin team are doing ! 😘 🇫🇷

Environment

React-admin version: 4.10.2
React version: 18.2
Browser: Chromium

slax57 commented 1 year ago

Hi! Thank you so much for your kind words, and the positive feedback! We are truly delighted to know you are enjoying react-admin :slightly_smiling_face:

Now, regarding your issue, I'm afraid I don't fully agree. react-admin is designed to let you provide any version of the third-party peer dependencies we are using (provided they meet the minimum requirement of course), to allow for greater flexibility and better integration alongside other existing React apps.

We try our hardest not to introduce any breaking changes in our minor and patch updates, to comply with semver, and we expect third-party libs to do the same.

To me, both examples you give are examples of the third-party libs not strictly following semver, as they seemingly introduced BCs into minor updates, but I don't consider it to be react-admin's responsibility. You can imagine them fixing the (mistakenly introduced) BC in a later release, which would land in your app just like it landed in the first time, without requiring any change from RA's side.

Of course, in case a third-party lib introduces an intentional BC they are willing to keep, in this case RA will need to be updated and you may of course open a issue to warn us about it in case we don't notice. But to me this is rather the exception than the rule.

TLDR; we are following semver and expect our dependencies to do the same, so we should be able to keep ^ instead of ~.

Two more things I'd like to mention:

Removing the yarn.lock and running yarn install is actually equivalent to running yarn up for all dependencies. This should be a manual step and should be followed by tests to make sure your app is still working. The rest of the time, you should keep the yarn.lock file in your versioning system and don't delete it.
Adding a "resolutions" section to your package.json seems like a perfectly valid solution to me if you want to prevent libs updates

Hence, I'll be closing this issue, unless @fzaninotto or @djhi have different thoughts on this.

In any case, thanks for sharing this, and again, thank you so much for your appreciated feedback :heart:

bramis commented 1 year ago

HI @slax57 ! Thank you for you answer 😊

I agree, let say react-router[-dom] issue was "special" and not following semver which introduced a BC for react-admin, but another issue was with ra-ui-materialui third-party library which is maintain by the Marmelab - React Admin team as well as ra-core and some ra-* libraries. They also seems to follow react-admin's versions to always have the same version value, so, in my case, every ra-* dependencies in my node_modules was recently updated to version 4.11.* because of ^ use which made my confused this react-admin version wasn't changed on my side. (packages concerned in my case: ra-core, ra-i18n-polyglot, ra-language-english and ra-ui-materialui). That's maybe where the use of ~ or a fixed version should be appropriate.

Also, to clarify something (be sure we're aligned on the comprehension) my project mostly use react-admin, the issues provided didn't happened on a custom code or something outside react-admin scope/context.

react-admin is designed to let you provide any version of the third-party peer dependencies we are using (provided they meet the minimum requirement of course), to allow for greater flexibility and better integration alongside other existing React apps.

You're right and, in a nutshell, the use of peerDependencies in package.json allow this flexibility and provide the minimum requirement where in the other way the dependencies are here to assure the library is working 😊 But I assume that we could have a different POV of a library deps. management, that's ok 😊

Removing the yarn.lock and running yarn install is actually equivalent to running yarn up for all dependencies. This should be a manual step and should be followed by tests to make sure your app is still working. The rest of the time, you should keep the yarn.lock file in your versioning system and don't delete it.

Indeed, we decided to remove the yarn.lock because it caused issues on my side and I agree, this is an arbitrary choice but this is something that could happen in a local development environment also and in that way, don't you think it should maybe be considered by react-admin library? 😊

I'll keep using resolutions in my package.json if you still consider this issue is not one of them and doesn't need to be address.

Have a great day! 🤗

fzaninotto commented 1 year ago

Hi Bramis,

And thanks for the valuable feedback.

If the TypeScript compilation of existing code fails with an update, this is a mistake on our part. Could you please post the details about the FunctionField issue you're describing?

As for the external dependencies, I agree with @slax57. We expect our dependencies to follow semver.

I also tend to agree that we could restrict internal dependencies (e.g. from react-admin to ra-ui-materialui) to bug fix releases only (~ instead of ^). I'm afraid there may be side effects, though. But let's reopen this issue to try it for a few releases.

slax57 commented 1 year ago

but another issue was with ra-ui-materialui third-party library which is maintain by the Marmelab - React Admin team

Actually I read your issue to quickly and thought you were talking about @mui/material, my bad! :sweat_smile:

slax57 commented 1 year ago

If the TypeScript compilation of existing code fails with an update, this is a mistake on our part. Could you please post the details about the FunctionField issue you're describing?

Can you confirm the issue you've run into was the same as this one?

bramis commented 1 year ago

Hi everyone, Sorry for the delay, yesterday was busy day for me 😅 Thanks @slax57 & @fzaninotto for your answers, @slax57 yes, the issue I've run into was the same as this one 👌

Dreamsorcerer commented 9 months ago

Just merging from my duplicate issue. I encountered the same issue. It's surprising that when you pin to a specific version of react-admin that you actually end up with all of the code from the latest version, which is caused by the ^ version numbers of subpackages. (Given that the react-admin package itself has almost no code and looks like it gets changed about once a year, it is literally installing 100% of the same code as the latest version).

Workaround at the moment is to pin all the subpackages (and hope new ones aren't added without me noticing) and setup a grouped Dependabot update: https://github.com/aio-libs/aiohttp-admin/pull/860/files

marmelab / react-admin