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

Rebuilding schema results in failure due to default value analysis #787

Closed
oliversalzburg opened this issue Feb 19, 2021 · 3 comments
Closed
Assignees
Labels
Bug 🐛 Something isn't working Community 👨‍👧 Something initiated by a community Solved ✔️ The issue has been solved
Projects
Milestone

Comments

@oliversalzburg
Copy link
Contributor

oliversalzburg commented Feb 19, 2021

Describe the Bug
When buildSchema has been called once, future calls result in an exception being thrown due to default value conflicts. Examples of the error are:

The 'created' field of 'ArchiveInput' has conflicting default values. Default value from decorator ('Fri Feb 19 2021 10:58:17 GMT+0100 (Central European Standard Time)') is not equal to the property initializer value ('Fri Feb 19 2021 10:58:27 GMT+0100 (Central European Standard Time)').

The 'cordova' field of 'SupportArchiveInput' has conflicting default values. Default value from decorator ('[object Object]') is not equal to the property initializer value ('[object Object]').

To Reproduce
The root cause seems to be a = new Date() initializer in the target class. As a new instance of the class is constructed during schema generation, to retrieve default values from the class instance, the second call has a more recent timestamp.

@InputType("ArchiveInput")
@ObjectType()
export class Archive {
	@Field(is => Date)
	created = new Date();
}

This also affects other cases where a complex default object is assigned with x = new Complex() in the class.

I created a full repro at https://github.com/oliversalzburg/double-build-repro

Expected Behavior
buildSchema calls should be idempotent.

Additionally, assigning a default value in the graph through this mechanism wasn't intentional. It was purely meant as a measure to initialize locally constructed instances of that class.

Environment (please complete the following information):

  • OS: Windows 10
  • Node v14.15.4
  • Package version 1.1.1
  • TypeScript version 4.1.5

Additional Context

const inputInstance = new (inputType.target as any)();

@MichalLytek MichalLytek added Bug 🐛 Something isn't working Community 👨‍👧 Something initiated by a community labels Feb 21, 2021
@MichalLytek MichalLytek self-assigned this Feb 21, 2021
@backbone87
Copy link

imho this is a design problem of your API. i faced the same in my current project. you never want to use "dependant values" as default values. while type-graphql could avoid this specific error, it is still strange, if you compare a schema generated (via introspection) an hour ago with a current one. they should be equal, but they are not.

possible solutions:

  • make it nullable and add the fallback in your resolver, while documenting that passing null, will result to the "current" date
  • create a new scalar which allows the use of "now" as a placeholder for "the current date".

in my API i ended up creating a "RelativeDateTime" which i use in almost all input positions, it supports "now" (which uses the query date), ISO duration (which is resolved to a date time by adding it to the query date) and absolute ISO dates.

@oliversalzburg
Copy link
Contributor Author

if you compare a schema generated (via introspection) an hour ago with a current one. they should be equal, but they are not.

I fully agree.

This behavior is unexpected, as I had no intention to even affect the API in this manner. All I wanted to achieve, was that I have a default value when I locally construct an instance of the class, like in a test. There is a specific defaultValue setting that I can and do use when I want to set default values. I didn't expect any magic to happen beyond that.

Type-GraphQL, by applying this default value detection magic, prevents this use case entirely. I end up declaring an input schema (where the input value is nullable and then set in a resolver), an output schema (where the value isn't nullable) and a class to internally represent the same schema, but which sets default values upon construction. This is something I was hoping to avoid by using the library in the first place.

I'm happy to accept that this 3-model-approach is ultimately the correct one for what I'm trying to achieve. I just wasn't expecting this and the root cause is unsettling. I don't feel in control of what's happening. I'd much rather like to disable this default value detection entirely.

@MichalLytek
Copy link
Owner

Closing via 5f864a2 🔒

Opening a separate ticket for tracking the implicit default value magic issue: #793

@MichalLytek MichalLytek added the Solved ✔️ The issue has been solved label Feb 28, 2021
@MichalLytek MichalLytek added this to the 1.x versions milestone Feb 28, 2021
@MichalLytek MichalLytek added this to Backlog in Board via automation Feb 28, 2021
@MichalLytek MichalLytek moved this from Backlog to Done in Board Feb 28, 2021
@MichalLytek MichalLytek modified the milestones: 1.x versions, 1.2 Nov 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐛 Something isn't working Community 👨‍👧 Something initiated by a community Solved ✔️ The issue has been solved
Projects
Board
  
Done
Development

No branches or pull requests

3 participants