support defaultValue for input types

[benawad]

fix: #53
improved on: #141
I'm happy to iterate on this further if any changes are needed.

[codecov]

Codecov Report

Merging #203 into master will increase coverage by 0.06%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #203      +/-   ##
==========================================
+ Coverage   95.33%   95.39%   +0.06%     
==========================================
  Files          59       60       +1     
  Lines         964      978      +14     
  Branches      166      169       +3     
==========================================
+ Hits          919      933      +14     
  Misses         44       44              
  Partials        1        1

Impacted Files	Coverage Δ
src/schema/utils.ts	82.35% <ø> (ø)	⬆️
src/schema/schema-generator.ts	97.1% <100%> (+0.14%)	⬆️
src/errors/ConflictingDefaultValuesError.ts	100% <100%> (ø)
src/errors/index.ts	100% <100%> (ø)	⬆️
src/helpers/types.ts	95% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7ca81ec...d345838. Read the comment docs.

[MichalLytek]

Looks ok but it's just a beginning.
Please take a look at this comment:
#141 (review)

[benawad]

What is Support for initializers in classes ("reflection") - creating an instance of the class and checking the value of the properties to detect default values?

[MichalLytek]

@benawad

@InputType()
class SampleInput {
  @Field()
  implicitDefaultField: string = "implicitDefaultFieldValue"

  @Field({ defaultValue: "explicitDefaultFieldValue" })
  explicitDefaultField: string;
}

input SampleInput {
  implicitDefaultField: String = "implicitDefaultFieldValue"
  explicitDefaultField: String = "explicitDefaultFieldValue"
}

[benawad]

How should I go about doing that?

[MichalLytek]

@benawad
Just by "creating an instance of the class and checking the value of the properties to detect default values" 😃

class SampleInput {
  normalField: string;
  stringFieldWithDefault: string = "defaultValue"
}

const instance = new SampleInput();

const defaultsMap = Object.keys(instance).map(fieldName=> ({
  fieldName,
  defaultValue: instance[fieldName],
}));

console.log(defaultsMap);

[benawad]

I was able to get it working for input objects, but I'm having trouble figuring out how to do it with args

[MichalLytek]

@benawad
You need to do the same things as with input - just create the instance in mapArgFields, read the defaults and attach them to args[field.schemaName] config:

const instance = new (argumentType.target as any)();

[MichalLytek]

We should also test some edge cases, like overwriting default values in child classes (implicit and by decorator option):

class Base {
  prop = "baseProp"
}

class Child extends Base {
  prop = "childProp"
}

And I think that we should report to user when implicitDefaultValue has the value and field.typeOptions.defaultValue has the value and they're not equal to each other.
Now it's just silently overwrites the default value from decorator.

[benawad]

Is it possible to detect the default values for function parameters?

@Arg("implicitDefaultStringArg")
implicitDefaultStringArg: string = "implicitDefaultStringArgDefaultValue",

[MichalLytek]

It is possible by calling .toString() do get the source code and parse it but it might not work when transpiled to ES3/ES5 as it's a runtime operation. Also it's hard to create a good regex that will handle all the combination of function declarations or when the value is a variable, not an explicit string, bool or number. That's why I don't read the arg name using that way, so you have to pass it to the decorator.

[benawad]

Would you like me to make any other changes?

[MichalLytek]

@benawad

Would you like me to make any other changes?

Yes, we need to add info about all this things in docs 😃
https://19majkel94.github.io/type-graphql/docs/resolvers.html

• defaultValue in @Arg decorator
• defaultValue in @Field decorator (only for @InputType and @ArgsType)
• initialization of property affects default value in schema
• inheritance of default values
• warning about conflict between decorator and and initializer default value

Also, examples and docs might need some updates when there's a default value together with { nullable: true } decorator's option.

And how do we handle the edge case when parent class has property initializer but child try to overwrites it with decorator option? Does it throw error?
And what about when parent has decorator option and child overwrites it with property initalizer?
Do we even test the simple inheritance with no overwrites? 😕

[benawad]

"And how do we handle the edge case when parent class has property initializer but child try to overwrites it with decorator option? Does it throw error?"
"And what about when parent has decorator option and child overwrites it with property initalizer?"

I think we should take the child defaultValue in both cases without throwing an error because they could be inheriting from a class from a library or something and not know (and don't care) how thaat class set the default value.

"Do we even test the simple inheritance with no overwrites? 😕"

Nope, and I just added a test for it and it failed 😂. Where in the code do you handle inheritance?

I'll start working on the docs next.

[MichalLytek]

Where in the code do you handle inheritance?

Before #183 it's all over the place 😆
Now I've realized that @Authorized also might not work with inheritance 😞

Args classes inheritance is here:
https://github.com/benawad/type-graphql/blob/d7659fb0d446d3a35ba7943b7f82a60bb939d3bb/src/schema/schema-generator.ts#L453-L460

Input types inheritance is here:
https://github.com/benawad/type-graphql/blob/d7659fb0d446d3a35ba7943b7f82a60bb939d3bb/src/schema/schema-generator.ts#L330-L336

I think we should take the child defaultValue in both cases

Right! Make sure that the tests are covering this cases 😉

Thanks and keep going! 💪

[MichalLytek]

@benawad

start working on docs

Are you going to finish this up or should I checkout your branch and add some changes there?

[benawad]

I'm not sure when I'll get time to finish this up, so it would be great if you want to.

[MichalLytek]

@benawad
I think that we are ready to go with this feature.
Can you take a look at my changes and review them? 😉

[benawad]

I'll take a look at it tonight

[benawad]

Looks good