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

TypePolicies #32

Merged
merged 1 commit into from
Aug 19, 2020
Merged

TypePolicies #32

merged 1 commit into from
Aug 19, 2020

Conversation

jeddeloh
Copy link
Owner

@jeddeloh jeddeloh commented Aug 15, 2020

@jfrolich Here's some proper TypePolicies.t and pagination helpers. It all turned out rather less elegant than I would have liked. Rather than go into detail, I thought I'd see if you had any initial feedback.

EXAMPLES/src/caching/Pagination.re shows the basic ergonomics

@jeddeloh jeddeloh mentioned this pull request Aug 15, 2020
let cache =
ApolloClient.Cache.InMemoryCache.(
make(
~typePolicies=[|
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not work with objects and have a more 1:1 binding?

Copy link
Owner Author

@jeddeloh jeddeloh Aug 16, 2020

Choose a reason for hiding this comment

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

I just didn't see any way to express the types with objects at all besides doing Js.t('anything). This uses Js.Dict.t under the hood. I opted to just forego requiring the user to Js.Dict.fromArray in favor of passing the array directly. Do you think it would be better to require an actual Js.Dict.t for clarity?

Copy link
Owner Author

Choose a reason for hiding this comment

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

One thing that stinks as well with this implementation is that you can't just choose to bypass that and use Js.t yourself because it is expecting specific variants to represent the unions in typescript. 🤷

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is pretty nice actually!

@jeddeloh jeddeloh merged commit 38d5b88 into master Aug 19, 2020
@jeddeloh jeddeloh deleted the type-policies branch August 19, 2020 18:39
@jeddeloh jeddeloh mentioned this pull request Aug 19, 2020
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