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

feat(math): new math package and useProjection function #1224

Merged
merged 5 commits into from
Jul 6, 2022

Conversation

Kingwl
Copy link
Contributor

@Kingwl Kingwl commented Feb 8, 2022

No description provided.

@wheatjs
Copy link
Member

wheatjs commented Feb 8, 2022

Interesting, a few things I'd like to mention. In VueUse we prefer to return an object rather than an array. Secondly I've noticed you've created a couple PR's (#1223) for math related functions. I'm wondering if we would rather have a @vueuse/math library as some of the names might confuse users thinking it is something else. @antfu any input on a new package for these types of functions?

@Kingwl
Copy link
Contributor Author

Kingwl commented Feb 10, 2022

I'm Ok with that. Would you mind tell me what should I do?

@antfu
Copy link
Member

antfu commented Feb 10, 2022

Let's have a @vueuse/math then. Anyone interested in it feel free to kickstart it and send a PR

@Kingwl
Copy link
Contributor Author

Kingwl commented Feb 10, 2022

I'm not a big fan about @vueuse/math:

  1. Currently, other packages all have third party dependencies (electron, firebase, etc). But the Math does not.
  2. If we have /math, why don't we have something like /events, /convertion, /encoding, or something like that ? It's seems a bit overkill.

How about just a new category called Math?

@wheatjs
Copy link
Member

wheatjs commented Feb 10, 2022

We have @vueuse/shared and @vueuse/components which have the same dependencies as @vueuse/core. The reason I think it is a good idea is that both the PR's you have opened useSlidingWindow and useProjection have names that people might confuse as other functions. If they are under a math package it will be more clear and we will be less likely to run into similar confusion or possible conflicts in the future

@Kingwl
Copy link
Contributor Author

Kingwl commented Apr 27, 2022

Have create a new package /math.

@antfu antfu requested a review from wheatjs April 27, 2022 11:49
@Kingwl
Copy link
Contributor Author

Kingwl commented Jul 4, 2022

ping 👀

@wheatjs
Copy link
Member

wheatjs commented Jul 4, 2022

Sorry I've been pretty busy recently so I haven't had as much time to review PRs. This looks good to me though!

@antfu antfu changed the title feat(useProjection): add function feat(math): new math package and useProjection function Jul 6, 2022
@antfu antfu added this to the 9.0 milestone Jul 6, 2022
@antfu antfu changed the base branch from main to next July 6, 2022 18:16
@antfu antfu merged commit 09be6ee into vueuse:next Jul 6, 2022
This was referenced Jul 10, 2022
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.

None yet

3 participants