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

Add Circuit.find and findAll methods to replace other get methods #1278

Open
wants to merge 4 commits into
base: model_refactor_api
Choose a base branch
from

Conversation

LeonMontealegre
Copy link
Member

I'm very much still unsure if this change is worth making, my solution feels hacky but unfortunately it's the only way I found to make it work.

The problem resides in that apparently templated methods like the find method cannot be acted on by something like ToDigital as it will strip it of it's templating. I think this is related to something called "higher kinded types", which is just a fundamental TS limitation.

That means each sub-project of the API will have to declare the find/findAll methods explicitly with their types which might not be that big of a deal and we can further make some core utility for typing it explicitly but it's not ... great

If these end up being the only instances of templated methods then it might just simply not be worth the effort but idk.

Not sure it even really cleans up the usage much (at least currently), but theoretically it could allow for some nice fancy querying when the queries are more complex than "get all components".

Thoughts?

@LeonMontealegre LeonMontealegre changed the title Added Circuit.find and findAll methods to replace other get methods Add Circuit.find and findAll methods to replace other get methods May 12, 2023
@TGCrystal
Copy link
Contributor

I feel like we had examples where this would be nice but I can't remember them now. Can you remember any more advanced use cases that make this worthwhile?

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

Successfully merging this pull request may close these issues.

None yet

2 participants