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 attributes to QueryOptions #451

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add attributes to QueryOptions #451

wants to merge 1 commit into from

Conversation

kldeb
Copy link

@kldeb kldeb commented Nov 28, 2024

No description provided.

Copy link

netlify bot commented Nov 28, 2024

Deploy Preview for electrodb-dev canceled.

Name Link
🔨 Latest commit 2529989
🔍 Latest deploy log https://app.netlify.com/sites/electrodb-dev/deploys/6747d281e2608600089a6e2d

@kldeb
Copy link
Author

kldeb commented Dec 15, 2024

@tywalch is there something else required?

@tywalch
Copy link
Owner

tywalch commented Dec 15, 2024

Hi @kldeb 👋

Sorry I haven't had a chance to respond. Thanks for putting together a PR! Unfortunately, the typing added to this PR does not have a valid typescript syntax. There is a reference to a non-existent Attributes type. If you check out how this is accomplished for queries, you'll see it requires more legwork to get the list of valid attribute names for the options. Lastly, there is a pattern of type tests for additions like this, and tests would be required to merge a change like this.

I appreciate your willingness to contribute to the project; changes like this can be challenging for any outside contributor because of the library's approach to strong inference.

@kldeb
Copy link
Author

kldeb commented Dec 16, 2024

Thanks @tywalch. I was looking for a typecheck command to run. I've since found test:types. I will try to find some time to dig in deeper. This is an intense level of typing!

@tywalch
Copy link
Owner

tywalch commented Dec 28, 2024

Thanks @tywalch. I was looking for a typecheck command to run. I've since found test:types. I will try to find some time to dig in deeper. This is an intense level of typing!

@kldeb Yeah "insane" in the way that only an insane person would have written it 🤪

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