Dependencies

[CaitlinSweeney]

Hi @csandman!

This pkg is great, thanks for creating it! I'm using in my custom Chakra-Ui library, which I import into a couple of my applications. I had to customize a few of them because of the designs, and needing to create custom components, etc.

Currently, I am not using the entire @chakra-ui/react pkg as a peerDependency in my packages. I just realized you have it set as a peerDependency. I forked the repo, but was wondering if it would be possible to only replace the necessary packages you are using? Update the peerDependencies & devDependencies ? I created a branch and updated the affected files, but figured a discussion before contributing a PR would be preferable.

"peerDependencies": {
  "@chakra-ui/form-control": "1.5.8",
  "@chakra-ui/icon": "2.0.5",
  "@chakra-ui/layout": "1.7.6",
  "@chakra-ui/menu": "1.8.8",
  "@chakra-ui/spinner": "1.2.6",
  "@chakra-ui/system": "1.11.1",
  "react": ">=16.8.6",
  "react-dom": ">=16.8.6"
},

Thanks!

[csandman]

I tested out your changes and I think its a good idea! I would accept a PR for this if it included only the changes for the peerDeps and package imports and not the other customizations, to keep it single purpose.

However, I notice all of the peer dependencies are pinned at absolute versions, which would inevitably lead to a project installing duplicate chakra dependencies at different versions if someone has a different version of @chakra-ui/react installed (at least I think, I still don't understand the inner workings of NPM as well as I should). I was thinking something like this would probably give the best range of compatibility:

"peerDependencies": {
  "@chakra-ui/form-control": "^1.0.0",
  "@chakra-ui/icon": "^2.0.0",
  "@chakra-ui/layout": "^1.0.0",
  "@chakra-ui/menu": "^1.0.0",
  "@chakra-ui/spinner": "^1.0.0",
  "@chakra-ui/system": "^1.0.0",
  "react": ">=16.8.6",
  "react-dom": ">=16.8.6"
},

The safest option would of course be to check every version of these sub packages to see if any version causes bugs, but if the semver is done correctly for Chakra UI, this should be mostly safe. And I'd rather avoid making the end user install extra package versions for that remote possibility. And of course if any issues with specific versions do crop up, I can always pin them more specifically later.

I'm open to discussion on this however, if you have any more insight on the topic!

---

On a different note, I'm curious about the other changes in the PR you closed. Did you look into passing a custom components object, or using the chakraStyles prop to make the changes you wanted? It seems like most of these changes could be accomplished by using one of those and you wouldn't have to maintain a fork. That is, once these dependency changes are in. And if your changes couldn't be made using these props, were there any specific limitations that kept you from accomplishing what you were trying to?

And besides the customization changes that are specific to your UI library, are there any other "correctness" changes that you think should be merged into this package? There were a lot of small changes in there, so I couldn't tell from a quick glance if there was anything else that would good to merge in.

[CaitlinSweeney]

Thanks! Sorry, I accidentally put a PR to the main repo instead of my forked repo to test 🙄 (I quickly closed it). It had my custom components in there, which were minor UI, icons, etc. Not meant for the main chakra-react-select pkg.

To be perfectly honest, my chakraComponents are not that different, other than the imports (since I'm not importing @chakra-ui/react). I was back and forth with the best way to remove the need to create any css/styling in the consumer apps. Since I am import the chakra-react-select components into two separate apps. Which again...really wasn't too much in the end. Other than the icons.

The only thing I added the might be of use, was a prop added to a Box component implementations for the Option. Not sure if the recent @chakra-ui packages bumps added more types. I'll put a proper PR in and make a comment in any places you might want to include. Thanks!

[csandman]

No worries about the accidental PR haha, I kind of figured it wasn't intended to be merged, it still made me curious about your changes though.

And yeah, once your PR is merged, you could consider look into adding the styles/replacing the components with the chakraStyles and components props respectively. It could be a pain if you want to share them between apps but it would probably be easier to keep up with updates to this package. Either way its up to you, just food for thought!

[CaitlinSweeney]

However, I notice all of the peer dependencies are pinned at absolute versions, which would inevitably lead to a project installing duplicate chakra dependencies at different versions if someone has a different version of @chakra-ui/react installed (at least I think, I still don't understand the inner workings of NPM as well as I should). I was thinking something like this would probably give the best range of compatibility

The safest option would of course be to check every version of these sub packages to see if any version causes bugs, but if the semver is done correctly for Chakra UI, this should be mostly safe. And I'd rather avoid making the end user install extra package versions for that remote possibility. And of course if any issues with specific versions do crop up, I can always pin them more specifically later.

• Yes, as a dev who is constantly monitoring my @chakra-ui deps, they have to be updated together, or shit breaks 😂
  @chakra-ui release very frequently, since it's mainly patches from updates to their main pkg like @chakra-ui/system.
  I'm not sure if you are using dependabot. I personally do not rely on it, since it will try to bump my @chakra-ui deps individually. I usually watch the @chakra-ui discord/repo for releases, and bump them all together with lernaupdate.

[csandman]

Yeah I understand your point about them having to be updated together, my main point was just that it shouldn't be this package's job to ensure that the versions of all of the dep packages are on compatible versions with each other. Anyone using this package already has to have a fully functional Chakra setup in order for this package to work, and if they don't the rest of their app will be breaking anyway. Especially since this package's use of Chakra props is pretty limited (for the most part) to a small amount of Chakra props that shouldn't be affected much without a major version change.

For example, if someone wants to use @chakra-ui/react version 1.5.3 (for whatever reason) and all of the accompanying sub packages, that should work fine with this package without it having to install duplicates of all of the sub packages at more recent versions.

I made a PR to your PR branch updating the package to accept older versions and explained my reasoning there, but I did also test by pinning all of the packages on their minimum version to see if it still worked right, and tweaked the versions until it did.

[CaitlinSweeney]

Your are right, I have not contributed to OS before, so appreciate you pointing out the obvious, thanks 👍🏻

[CaitlinSweeney]

Thanks so much for your detailed description in your PR!

[csandman]

Well, ever since I made this change my head has been spinning. I thought it would be all fine because most of the newer versions of npm install peer dependencies automatically, but I forgot about yarn. And because most people using this package probably have @chakra-ui/react installed instead of the actual sub-components, yarn throws warnings about missing peer dependencies, and for some versions of yarn, doesn't work at all without them. This was brought to my attention in #78.

For a while I was considering reverting to having a peer dependency of @chakra-ui/react but I didn't want to break your use case, but I wanted it to work for the majority as painlessly as possible. Finally I looked at how Chakra handles this in their own components and realized they have them all in dependencies, not peer dependencies.

For example:

// @chakra-ui/close-button package.json
   
{
  "dependencies": {
    "@chakra-ui/icon": "3.0.0",
    "@chakra-ui/utils": "2.0.0"
  },
  "devDependencies": {
    "@chakra-ui/system": "2.0.0",
    "react": "^18.0.0"
  },
  "peerDependencies": {
    "@chakra-ui/system": ">=2.0.0-next.0",
    "react": ">=18"
  }
}

And it finally clicked! I realized this solves the issue perfectly without adding any additional bloat, because npm and yarn will de-duplicate dependencies anyway! Since making this change, the installation process is so much simpler for everyone and it doesn't break your use case.

And I realized this just in the nick of time, because since Chakra UI released their v2, my installation was entirely broken because of using peerDependencies instead of dependencies. So this really killed two birds with one stone.

These changes are both in v3.3.5 and v4.0.0-rc.1.