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

Support nested fragments. #23

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

DarryQueen
Copy link

Fixes #14 (and thanks to @Warxcell for the test!).

@Warxcell
Copy link

Can we move this forward? :)

@DarryQueen
Copy link
Author

@clentfort?

@ericbf
Copy link

ericbf commented Jun 5, 2023

@clentfort thoughts on this?

@@ -77,7 +88,7 @@ function mapScalar(data: any, path: PropertyKey[], map: ScalarMapping) {
if (Array.isArray(newSubData[segment])) {
const subPath = path.slice(index + 1);
newSubData[segment] = newSubData[segment].map((subData: unknown) =>
mapScalar(subData, subPath, map)
mapScalar(subData, subPath, mapping)
Copy link

Choose a reason for hiding this comment

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

Not a fault in either PR, but note that the renaming of map to mapping here would need to be resolved with this one in #23 if both get merged:
https://github.com/clentfort/urql-custom-scalars-exchange/pull/25/files#diff-a2a171449d862fe29692ce031981047d7ab755ae7f84c707aef80701b3ea0c80R73

When merging the two locally I was surprised to find tests fail, and it was just that tiny little thing.

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.

Doesn't handle nested Fragments
4 participants