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

Consider: Rename .grab to .select #29

Open
gksander opened this issue Jan 19, 2023 · 2 comments
Open

Consider: Rename .grab to .select #29

gksander opened this issue Jan 19, 2023 · 2 comments
Assignees

Comments

@gksander
Copy link
Contributor

gksander commented Jan 19, 2023

.grab as a method name is a bit odd, and is an artifact of the early experimental phases of the project. I'd like to phase that out by:

  • introducing a .select method that shadows .grab.
  • update documentation to use .select instead of .grab
  • add a console warning to .grab calls, pointing the user to .select
  • in the future, fully deprecate .grab and remove it from the codebase.

Also consider overloading this function and join grabOne's functionally with it.

@gksander gksander changed the title Rename .grab to .select Consider: Rename .grab to .select Jan 20, 2023
@nonphoto
Copy link

nonphoto commented Jan 22, 2023

grabOne might be called property or prop (as in lodash), if you're looking for suggestions. I think it makes sense to keep it as it's own function so it's clear what its doing at a glance.

@littlemilkstudio
Copy link
Contributor

Is this still open for discussion? or are things pretty settled? with the .select method existing here now based off of sanity's select() method it makes this a little more complex to reason about.

I guess there are some discrepancies in naming that we could unify if we want to. If we wanted to go more off of a GROQ convention, we could do .grab() -> .project() & leave .select() as is. Or we could choose to deviate from GROQ based naming conventions and rename .select() -> .conditional().

I guess another interesting api feature to help reason about things would be spread/merge, which would be nice to adopt here in the near future

Personally, I think it might be nice to base naming as closely to one api source as possible. Whether that be GROQ, Zod or whatever else.

Coming into the library, I found it helpful when methods were named more closely resembling their GROQ counter parts to make it easier to reason about the underlying GROQ query that's being built and referencing any relevant GROQ documentation. That being said, I agree that .project() sounds a little funky, and maybe it's fine that .grab() is our only deviation from GROQ naming conventions because of that 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants