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

fix: Handle null values for optional types #21

Merged
merged 1 commit into from
Dec 5, 2019

Conversation

gsamokovarov
Copy link
Contributor

@gsamokovarov gsamokovarov commented Dec 4, 2019

Without this change, I'm not able to get apollo-link-scalars running in my project.

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

This is a bugfix.

  • What is the current behavior? (You can also link to an open issue here)

apollo-link-scalars errors for valid null values of nullable types with errors like:

TypeError: String cannot represent a non string value: null

The errors are hard to track down as they occur during the Apollo client initialization.

  • What is the new behavior (if this is a feature change)?

apollo-link-scalars won't error on nullable scalar types anymore.

Without this change `apollo-link-scalars` will error for valid `null` values
of nullable types with hard to track down errors like:

```
TypeError: String cannot represent a non string value: null
```

With this change, I'm able to get `apollo-link-scalars` for my project.
@@ -169,7 +169,7 @@ describe("scalar returned directly from first level queries", () => {
expect.assertions(1);
});

it("override the scala resolvers with the custom functions map", done => {
it("override the scalar resolvers with the custom functions map", done => {
Copy link
Owner

Choose a reason for hiding this comment

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

d'oh… thanks :)

@@ -66,6 +66,8 @@ export class Parser {
fieldNode: ReducedFieldNode
): any {
const type = ensureNullableType(value, givenType, fieldNode);
if (isNone(value)) return value;
Copy link
Owner

Choose a reason for hiding this comment

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

good point. Originally I was thinking of allowing some custom parsing on nulls on scalars. Later I decided against it but I never went back and change the code. Nice one.

@eturino
Copy link
Owner

eturino commented Dec 5, 2019

@gsamokovarov nice one. Thanks!

@eturino eturino changed the title Handle null values for optional types feat: Handle null values for optional types Dec 5, 2019
@eturino eturino changed the title feat: Handle null values for optional types fix: Handle null values for optional types Dec 5, 2019
@eturino eturino merged commit 7499d45 into eturino:master Dec 5, 2019
@eturino
Copy link
Owner

eturino commented Dec 5, 2019

v0.1.4 released with this fix

@gsamokovarov
Copy link
Contributor Author

Thank you for the quick reaction!

@eturino
Copy link
Owner

eturino commented Dec 6, 2019

much obliged

@gsamokovarov gsamokovarov deleted the parse-nullables branch December 6, 2019 13:21
@eturino
Copy link
Owner

eturino commented Dec 7, 2019

@all-contributors please add @gsamokovarov for bug, tests and code

@allcontributors
Copy link
Contributor

@eturino

I've put up a pull request to add @gsamokovarov! 🎉

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