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

[Discussion] Convert to using optional records? #151

Open
illusionalsagacity opened this issue Feb 3, 2023 · 1 comment · May be fixed by #154
Open

[Discussion] Convert to using optional records? #151

illusionalsagacity opened this issue Feb 3, 2023 · 1 comment · May be fixed by #154
Labels
help wanted Extra attention is needed

Comments

@illusionalsagacity
Copy link
Contributor

It'd likely cut down on the size overhead of the library, and it certainly would look more like the TS/JS usage of apollo-client.

I also think it may be possible to reduce some of the runtime overhead, which in my experience has led to some odd edge cases where rescript-apollo-client will behave differently with it's return being used for effect dependencies to TS code. Some of this is unavoidable due to the graphql-ppx parsing, but it'd be nice to see where this could potentially reduce it.

That said, I kinda like the named parameters stylistically, and I am not sure if other users are still on ReScript or even bs-platform 9 still?

@jeddeloh
Copy link
Owner

jeddeloh commented Feb 3, 2023

Good or bad, I do think that using optional records is more in line with the philosophy of this library—that being that it's such a large surface area to cover, we want to be as close to the Apollo api as possible. Ideally, one could use their docs exclusively. Might be prudent to see some example usage first to confirm it works out like we would like. I'm not in a place right now where I'm going to take on a refactor that large, but I'm happy to help anyone who might.

There is also some defensive coding around exceptions that adds a lot of complexity for little gain that could be removed. There's long discussion about that somewhere on here :)

@jeddeloh jeddeloh added the help wanted Extra attention is needed label Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
2 participants