fix(graphql): ensure scalarsMap in buildSchemaOptions is respected

ssut commented 2 months ago

PR Checklist

Please check if your PR fulfills the following requirements:

[x] The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
[ ] Tests for the changes have been added (for bug fixes / features)
[ ] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior?

The scalarsMap in buildSchemaOptions was not used even though it was defined.

This PR also fixes issues like #3204 by correctly merging the scalarsMap, ensuring that duplicate ScalarsTypeMap entries are ignored by their type. e.g.

buildSchemaOptions: {
  numberScalarMode: 'integer',
  scalarsMap: [
    {
      type: Date,
      scalar: GraphQLDateTime,
    },
  ],
}

What is the new behavior?

This PR ensures that the scalarsMap in buildSchemaOptions is respected and correctly merged with the scalarsMap from ScalarsExplorerService.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

kamilmysliwiec commented 2 months ago

lgtm

kamilmysliwiec commented 2 months ago

Sounds like this PR might have introduced a regression https://github.com/nestjs/graphql/issues/3244

nestjs / graphql