Skip to content
This repository has been archived by the owner on Apr 15, 2020. It is now read-only.

Don't set input default values to null #15

Merged
merged 1 commit into from
Aug 23, 2019

Conversation

bconnorwhite
Copy link

TODO:

  • If this PR is a new feature, reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass

Copy link
Owner

@yaacovCR yaacovCR left a comment

Choose a reason for hiding this comment

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

Thanks for submitting a pull request!

Looks like this refactoring streamlines some code, but functionality unchanged. Is that about right?

But you have me worried there may be a bug lurking somewhere. Is anything breaking for you?

@bconnorwhite
Copy link
Author

The difference is between == and ===

Since we are comparing if(value == null) (not ===), if value is undefined, the default will be changed from undefined to null:

if (value == null) {
  return null;
}

Results in:
field(input: Type = null): ReturnType -> field(input: Type = null): ReturnType
field(input: Type): ReturnType -> field(input: Type = null): ReturnType

Whereas:

if (value == null) {
  return value;
}

Results in:
field(input: Type = null): ReturnType -> field(input: Type = null): ReturnType
field(input: Type): ReturnType -> field(input: Type): ReturnType

This causes issues with input types like this:

type UserWhereInput {
  attribute1: value,
  attribute2: value,
  attribute3: value
}

Say if an attribute is included, I want to return users with the matching value. If the default for each attribute is set to null, now I end up checking attribute2 == null and attribute3 == null even if I left those out of my query and just want to check equality on attribute1:

users(where: { attribute1: X }) {
  ...
}

@yaacovCR
Copy link
Owner

Got it. Looks good to me. Will merge as soon as I can. Thanks!

@yaacovCR yaacovCR closed this in 786a855 Aug 20, 2019
@yaacovCR
Copy link
Owner

Merged as v6.3.3.

@yaacovCR yaacovCR reopened this Aug 20, 2019
@yaacovCR yaacovCR merged commit 1c0f2c3 into yaacovCR:master Aug 23, 2019
yaacovCR pushed a commit that referenced this pull request Sep 22, 2019
Input fields without default values should not have default values of
null after transformation -- they should still have no default values.
Closes #15.
yaacovCR pushed a commit that referenced this pull request Sep 22, 2019
Input fields without default values should not have default values of
null after transformation -- they should still have no default values.
Closes #15.
yaacovCR pushed a commit that referenced this pull request Sep 22, 2019
Input fields without default values should not have default values of
null after transformation -- they should still have no default values.
Closes #15.
yaacovCR pushed a commit that referenced this pull request Sep 22, 2019
Input fields without default values should not have default values of
null after transformation -- they should still have no default values.
Closes #15.
yaacovCR pushed a commit that referenced this pull request Oct 3, 2019
Input fields without default values should not have default values of
null after transformation -- they should still have no default values.
Closes #15.
yaacovCR pushed a commit that referenced this pull request Oct 25, 2019
Input fields without default values should not have default values of
null after transformation -- they should still have no default values.
Closes #15.
yaacovCR pushed a commit that referenced this pull request Oct 25, 2019
Input fields without default values should not have default values of
null after transformation -- they should still have no default values.
Closes #15.
yaacovCR pushed a commit that referenced this pull request Nov 4, 2019
Input fields without default values should not have default values of
null after transformation -- they should still have no default values.
Closes #15.
yaacovCR pushed a commit that referenced this pull request Dec 31, 2019
Input fields without default values should not have default values of
null after transformation -- they should still have no default values.
Closes #15.
yaacovCR pushed a commit that referenced this pull request Dec 31, 2019
Input fields without default values should not have default values of
null after transformation -- they should still have no default values.
Closes #15.
yaacovCR pushed a commit that referenced this pull request Jan 8, 2020
Input fields without default values should not have default values of
null after transformation -- they should still have no default values.
Closes #15.
yaacovCR pushed a commit that referenced this pull request Jan 21, 2020
Input fields without default values should not have default values of
null after transformation -- they should still have no default values.
Closes #15.
yaacovCR pushed a commit that referenced this pull request Feb 27, 2020
Input fields without default values should not have default values of
null after transformation -- they should still have no default values.
Closes #15.
yaacovCR pushed a commit that referenced this pull request Mar 26, 2020
Input fields without default values should not have default values of
null after transformation -- they should still have no default values.
Closes #15.
yaacovCR pushed a commit that referenced this pull request Mar 26, 2020
Input fields without default values should not have default values of
null after transformation -- they should still have no default values.
Closes #15.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants