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

Moved required dependencies to dependencies #83

Conversation

gerhardadler
Copy link
Contributor

@gerhardadler gerhardadler commented Jun 5, 2024

When installing chakra-dayzed-datepicker, @chakra-ui/react, date-fns and dayzed are required.

You could install them seperately, but thats an additonal step for the user. It also clutters the users package.json unnecessarily.

In addition, I've had issues with depcheck (https://www.npmjs.com/package/depcheck) complaining I have unused deps. (Due to date-fns and dayzed not being used outside chakra-dayzed-datepicker in my program)

It would be easier if these are listed as direct dependencies, and that is the purpose of this PR.

@aboveyunhai
Copy link
Owner

aboveyunhai commented Jun 5, 2024

Make sense, except I wouldn't put the charka into dependencies since ppl must have chakra lib first before they knew this lib.
I also don't want them to install the entire chakra by install this package itself.
So devs have a clear idea what they are trying to install. I mean if they want to use the lib solely and not aware of it will carry out the entire chakra. Something must be wrong.

I will put the chakra back for you if you don't want to do it.

Fwiw, if you wonder why I have such a annoying install path initially, it's because at that moment I was attempting to separate the date library out of the core package, as they are very huge so ppl can choose whatever they want. but obviously it didn't work out (in a elegant way, not worth the path I attempted) and I wish ppl can just copy&paste the code instead of npm install. 😄

@gerhardadler
Copy link
Contributor Author

Sounds fair 👍️

@aboveyunhai aboveyunhai merged commit c4c5be6 into aboveyunhai:main Jun 6, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants