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

Override superfetcher to allow misordered and missing results #28

Merged
merged 2 commits into from Mar 22, 2023

Conversation

coyotesqrl
Copy link
Contributor

Adds a second arity to the def-superfetcher macro to take two functions: one to define how to match input ids with response objects, and one to define what to do if there is a missing response object

- Adds a second arity to the `def-superfetcher` macro to take two functions: one to define how to match input ids with response objects, and one to define what to do if there is a missing response object
@oliyh
Copy link
Owner

oliyh commented Mar 15, 2023

Hi,

Thanks for this contribution. I was wondering why you wouldn't just have this code in your fetching function, but then I guess the whole point of this macro is for convenience above all else.

I was wondering if some better documentation would have saved you some time tracking down the cause of your recent issue that we spoke about on Slack, particularly now that there is this extra choice of how you can meet the contract? If you agree it would be useful to see that in the readme.

Also would you be able to provide a test to exercise this code and serve as an example?

Many thanks, let me know if I can help in any way.
Cheers

@coyotesqrl
Copy link
Contributor Author

Hi,

Thanks for this contribution. I was wondering why you wouldn't just have this code in your fetching function, but then I guess the whole point of this macro is for convenience above all else.

I was wondering if some better documentation would have saved you some time tracking down the cause of your recent issue that we spoke about on Slack, particularly now that there is this extra choice of how you can meet the contract? If you agree it would be useful to see that in the readme.

Also would you be able to provide a test to exercise this code and serve as an example?

Many thanks, let me know if I can help in any way.
Cheers

I'd be happy to add a test and document. I probably won't get to it until early next week, but that shouldn't be a problem.

- Adds a second arity to the `def-superfetcher` macro to take two functions: one to define how to match input ids with response objects, and one to define what to do if there is a missing response object
- Adds unit tests
- Updates README
@@ -170,6 +170,12 @@ We can rewrite this using superlifter (see the [example code](https://github.com
(with-superlifter context
(s/enqueue! :pet-details (->FetchPet id))))
```
Note that when defining a Superfetcher as above, the number of outputs is
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oliyh Added this note to the README, but did not update the example. I started down that road, but it was starting to get ugly, as I started adding a whole parallel set of functions to support both superfetcher styles side-by-side with different queries.

If you'd like me to make that change - or have a different suggestion for the example project - let me know.

@oliyh
Copy link
Owner

oliyh commented Mar 22, 2023

Hello, this looks good to me, thank you for your contribution

@oliyh oliyh merged commit 2fec55a into oliyh:master Mar 22, 2023
1 check passed
@oliyh
Copy link
Owner

oliyh commented Mar 22, 2023

Available now in 0.1.4-SNAPSHOT

@coyotesqrl coyotesqrl deleted the superfetcher-customizable branch March 22, 2023 14:13
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

2 participants