feat(ts): Improves return types of QueryResolvers, MutationResolvers and <Model>Resolvers #6228

dac09 · 2022-08-18T11:31:15Z

This PR is a combination of fixes (for bugs I introduced during the strict-mode PR) and new features

What does this do?

In v2, we started mapping Prisma types (#5216) to QueryResolvers and MutationResolvers. This was a great improvement, but it required that all services return the full Prisma model and had a few other downsides. This PR addresses these:

A. Introduces a new utility type MergePrismaWithSdlTypes<PrismaUser, User> which is used in codegen to do the mapping. Which achieves the following cases:

Key Insight:
SDL is source of truth for KEYS
Prisma types is source of truth for VALUES (unless SDL-only field)

1. Return a subset of the fields 🆕
Let's say I modify my SDL to only return a subset of the fields in Post. In this case, I never want to return the createdAt property from GraphQl

export const schema = gql`
  type Post {
    id: Int!
    title: String!
    body: String!
-   createdAt: DateTime
  }

The Resolver types used to error out if I do not return createdAt in the resolver.

What's happening here is that SDL type gets updated, but the Prisma type doesn't (obviously, because we are mapping it, and the field still exists in the DB). The new helper will "pick" all the fields that are common in both the Prisma and the SDL Post type.

2. Return extra fields 🆕
In the opposite case, I'd like to return extra properties from my GraphQL API, that does not exist in the database/Prisma model. e.g.

export const schema = gql`
  type Post {
    id: Int!
    title: String!
    body: String!
+  sdlOnlyProperty: String!
  }

The generated types now take this into account.

3. Making certain fields optional in SDL, but still keeping them requried in Prisma schema 🆕

 type Post {
    id: Int!
    title: String!
-  body: String!
+   body: String # 👈 make body optional, even though prisma schema has body required

This will be handled as well, and your resolvers will not be required to return a body.

B Relation types 🆕
Before this PR, we didn't provide types for relations.

type Post {
	author: User # 👈 author could be anything, we didn't type it
    authorId: String # this was OK, because it came from Prisma

What happens now, in this example, we provide the types for Post.author, but we make this property optional, because its a relation. Redwood generates relation resolvers at the bottom of a service file

Making it optional solves the "infinite" nesting problem with graphql types i.e. Posts.author.posts.author.posts.... - but you still get type safety if you want to return the relation directly from your service, and not from the relation resolvers

B. Introduces removeNulls utility function 🆕
See relevant sections in the doc:

This is covered in detail in Prisma's docs, which we strongly recommend reading.
But the gist of it is that, for Prisma's create and update operations, you have to make sure nulls are converted to undefined from your GraphQL mutation inputs - unless the field is nullable in your Prisma schema.

This is because a null in graphql is not equivalent to null in Prisma.

nx-cloud · 2022-08-18T11:31:17Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 5d59f5f. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 14 targets

Sent with 💌 from NxCloud.

netlify · 2022-08-18T11:31:20Z

✅ Deploy Preview for redwoodjs-docs ready!

Name	Link
🔨 Latest commit	`5d59f5f`
🔍 Latest deploy log	https://app.netlify.com/sites/redwoodjs-docs/deploys/63076283964e330009e1586d
😎 Deploy Preview	https://deploy-preview-6228--redwoodjs-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

…and <Model>Resolvers

dac09 · 2022-08-22T15:21:30Z

packages/internal/src/generate/graphqlCodeGen.ts

-    ) => TResult extends PromiseLike<infer TResultAwaited>
-      ? Promise<Partial<TResultAwaited>>
-      : Promise<Partial<TResult>> | Partial<TResult>;`
+    ) => TResult | Promise<TResult>`


This return Resolver's ReturnType to the default. I introduced this in strict-mode pr and have subsquently realised this is a terrible idea.

dac09 · 2022-08-22T15:24:43Z

packages/internal/src/generate/graphqlCodeGen.ts

@@ -159,11 +185,11 @@ function getPluginConfig() {
    scalars: {
      // We need these, otherwise these scalars are mapped to any
      BigInt: 'number',
-      DateTime: 'string',
-      Date: 'string',
+      DateTime: 'Date | string', // @Note: because DateTime fields can be valid Date-strings too


Important change.

Redwood has built-in handling for Dates (because we https://www.graphql-scalars.dev/docs/scalars/date-time configured) - but our types were only allowing string. This was a misconfiguration, but not super visible, because we were mapping Prisma types anyway.

For DateTime fields you can send back Date but also a valid date-string. Note that when you set DateTime in your SDL, even if you format it yourself, it will convert it back to the RFC3339.

Note also that the code here only affects types we generate, I'm just highlighting what the behaviour actually is and updating the types to match it!

dac09 · 2022-08-22T15:26:05Z

packages/internal/src/generate/graphqlCodeGen.ts

@@ -22,15 +22,30 @@ import { getTsConfigs } from '../project'

 export const generateTypeDefGraphQLApi = async () => {
  const filename = path.join(getPaths().api.types, 'graphql.d.ts')
+  const prismaModels = getPrismaModels()
+  const prismaImports = Object.keys(prismaModels).map((key) => {


Moved importing Prisma types here, because we now use a "custom mapper" generic MergePrismaWithSdlTypes<> in the mapper object.

…string for api.

dac09 · 2022-08-22T17:44:50Z

packages/api/src/functions/dbAuth/DbAuthHandler.ts

@@ -239,7 +239,7 @@ type Params = {
 * //  key being used in dbAccessor in src/functions/auth.ts 👇
 * const getCurrentUser = async (session: DbAuthSession<User['id']>)
 */
-export interface DbAuthSession<TIdType = unknown> {
+export interface DbAuthSession<TIdType = any> {


@Tobbe had to set this to any again, because it complains about currentUser function being passed into the graphql handler in strict mode otherwise.

Next time I see this I'll get an itch to try to switch it to unknown again... How did you notice it was complaining? If I can check on my own next time it doesn't have to bounce in a PR :)

Just a tsc error in the projects graphql function.

It happens because findUnique (when trying to find the user) can return null I think

dac09 · 2022-08-22T17:45:36Z

docs/docs/typescript/strict-mode.md

    where: { id },
  })
 }
 ```

+### Relation resolvers in services


Getting a little messy here with this.

This part of the doc is half ts strict mode - and half "dealing with quirks of gql+prisma"

Not just this part, also the part above about removeNulls

dac09 · 2022-08-22T18:21:21Z

tasks/smoke-test/tests/storybook.spec.ts

@@ -172,7 +172,7 @@ storybookTest(
    await page.goto(STORYBOOK_URL)

    // Click Redwood link in left nav
-    await page.locator('css=[data-item-id=redwood--page]').click()
+    await page.locator('id=redwood--page').click()


I don't know why I had to change this now 🤷

I think I'm getting the same error in #6226, but it doesn't happen to any renovate PRs. Weird

Ah I take that back, was failing because I forgot to update a string here: 82ec512

I think it works in CI, but fails locally without this change.

Tobbe · 2022-08-24T04:59:45Z

docs/docs/typescript/strict-mode.md

    where: { id },
  })
 }
 ```

+### Relation resolvers in services


Not just this part, also the part above about removeNulls

docs/docs/typescript/strict-mode.md

Tobbe · 2022-08-24T05:11:22Z

packages/api/src/transforms.ts

+ * @param obj - Object to remove nulls from
+ * See {@link https://www.prisma.io/docs/concepts/components/prisma-client/null-and-undefined Prisma docs: null vs undefined}
+ */
+export const removeNulls = (obj: Record<number | symbol | string, any>) => {


dnull is recursive. Does this one have to be too?

Oh darn, maybe it should.

I've updated it and added a test, but I need to think more about the use cases for this a little. Nothing is ever easy.....

Tobbe · 2022-08-24T05:14:47Z

packages/internal/src/__tests__/__snapshots__/graphqlCodeGen.test.ts.snap

@@ -22,11 +24,11 @@ export type Scalars = {
  Int: number;
  Float: number;
  BigInt: number;
-  Date: string;
-  DateTime: string;
+  Date: Date | string;


I was curious to see what you decided to do here 🙂 Why did you have to keep the possibility of this being a string?

I explained it in a comment, but it got "outdated"

Important change.
Redwood has built-in handling for Dates (because we use graphql-scalars.dev/docs/scalars/date-time configured) - but our types were only allowing string. This was a misconfiguration, but not super visible, because we were mapping Prisma types anyway.
For DateTime fields you can send back Date but also a valid date-string. Note that when you set DateTime in your SDL, even if you format it yourself, it will convert it back to the RFC3339.
Note also that the code here only affects types we generate, I'm just highlighting what the behaviour actually is and updating the types to match it!

packages/internal/src/generate/graphqlCodeGen.ts

Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>

jtoar · 2022-08-26T07:50:08Z

@dac09 tried to help you out by handling the merge conflict in a snapshot file due to the jest 29 upgrade; let me know if t looks like I screwed something up and can revert.

packages/internal/src/generate/graphqlCodeGen.ts

…merge-mapped-types-codegen

packages/internal/src/generate/graphqlCodeGen.ts

This reverts commit 05645a0.

docs/docs/typescript/strict-mode.md

orta · 2022-08-30T10:22:32Z

packages/api/src/transforms.ts

@@ -35,3 +35,26 @@ export function normalizeRequest(event: APIGatewayProxyEvent): Request {
    body,
  }
 }
+
+// Internal note:  Equivalent to dnull package on npm, which seems to have import issues in latest versions


I got a build shipped for this: brielov/dnull#65

Thanks Orta! We might switch to dnull internally and keep the removeNulls export

dac09 added the release:feature This PR introduces a new feature label Aug 18, 2022

redwoodjs-bot bot assigned dac09 Aug 18, 2022

dac09 force-pushed the fix/merge-mapped-types-codegen branch from 1c74d4b to 9c9593f Compare August 18, 2022 11:59

dac09 changed the title ~~fix(ts): Improves return types of QueryResolvers, MutationResolvers and <Model>Resolvers~~ feat(ts): Improves return types of QueryResolvers, MutationResolvers and <Model>Resolvers Aug 22, 2022

feat(ts): Improves return types of QueryResolvers, MutationResolvers …

878f942

…and <Model>Resolvers

dac09 force-pushed the fix/merge-mapped-types-codegen branch from 3a6b563 to 878f942 Compare August 22, 2022 15:10

dac09 marked this pull request as ready for review August 22, 2022 15:18

dac09 commented Aug 22, 2022

View reviewed changes

dac09 added 7 commits August 22, 2022 16:26

Remove duplicate helper type

afc2e30

Missing files from commit

c614041

Update snapshots and tests

9194e10

Add docs on how to handle relation resolvers in strict mode

5a20076

Set DbAuthSession generic to any instead of unknown

462a180

update docs on relation resolvers

184ddc4

Only set DateTime scalar to string for the web side typegen. Date or …

f787261

…string for api.

dac09 force-pushed the fix/merge-mapped-types-codegen branch from 72d4aa3 to f787261 Compare August 22, 2022 17:42

Merge branch 'main' into fix/merge-mapped-types-codegen

40de555

dac09 commented Aug 22, 2022

View reviewed changes

Fix smoke test selector

187c1c3

dac09 commented Aug 22, 2022

View reviewed changes

dac09 mentioned this pull request Aug 23, 2022

Tracking issue for the next major (v3) #6064

Closed

18 tasks

dac09 added 2 commits August 23, 2022 17:10

Merge branch 'main' into fix/merge-mapped-types-codegen

bca651e

Merge branch 'main' into fix/merge-mapped-types-codegen

51d80bd

Tobbe reviewed Aug 24, 2022

View reviewed changes

Apply suggestions from code review

dcb0417

Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>

Merge branch 'main' into fix/merge-mapped-types-codegen

04fdfa7

jtoar and others added 2 commits August 26, 2022 17:06

Merge branch 'main' into fix/merge-mapped-types-codegen

dea3f1e

Tweak overriding resolver type doc

0b3968e

Tobbe reviewed Aug 27, 2022

View reviewed changes

packages/internal/src/generate/graphqlCodeGen.ts Show resolved Hide resolved

dac09 mentioned this pull request Aug 28, 2022

CodeGen: Try to generate prisma client if no models are found #6308

Merged

dac09 and others added 2 commits August 28, 2022 21:48

Make resolvers optional again | Check if prismaModel isnt generated

5bc5efd

Merge branch 'main' of https://github.com/redwoodjs/redwood into fix/…

7ddecce

…merge-mapped-types-codegen

Tobbe reviewed Aug 29, 2022

View reviewed changes

packages/internal/src/generate/graphqlCodeGen.ts Outdated Show resolved Hide resolved

dac09 added 8 commits August 29, 2022 11:42

Remove try catch

876bb20

Update relevant parts of the fixture

05645a0

Update full fixtures

4008403

Clarify docs on removeNulls

a72fd6e

Avoid Optionals...again

862eea3

Revert "Update relevant parts of the fixture"

c5af643

This reverts commit 05645a0.

Update comment about avoidOptionals

f93b64f

Revert changes due to avoidOptionals in templates

b1ad112

dac09 commented Aug 29, 2022

View reviewed changes

docs/docs/typescript/strict-mode.md Outdated Show resolved Hide resolved

Update docs/docs/typescript/strict-mode.md

a7cfaa2

orta reviewed Aug 30, 2022

View reviewed changes

Tobbe approved these changes Aug 30, 2022

View reviewed changes

Merge branch 'main' into fix/merge-mapped-types-codegen

cd65d48

dac09 enabled auto-merge (squash) August 30, 2022 11:03

dac09 merged commit 259fd77 into redwoodjs:main Aug 30, 2022

redwoodjs-bot bot added this to the next-release milestone Aug 30, 2022

dac09 deleted the fix/merge-mapped-types-codegen branch August 30, 2022 11:34

jtoar modified the milestones: next-release, v3.0.0 Sep 2, 2022

This was referenced Sep 5, 2022

[Bug?]: No type hints helping me when I add a field to my SDL, but forget to include it in the GQL resolver #5982

Closed

Document relation resolvers in strict mode #6344

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ts): Improves return types of QueryResolvers, MutationResolvers and <Model>Resolvers #6228

feat(ts): Improves return types of QueryResolvers, MutationResolvers and <Model>Resolvers #6228

dac09 commented Aug 18, 2022 •

edited by Tobbe

nx-cloud bot commented Aug 18, 2022 •

edited

netlify bot commented Aug 18, 2022 •

edited

dac09 Aug 22, 2022 •

edited

dac09 Aug 22, 2022

dac09 Aug 22, 2022

dac09 Aug 22, 2022 •

edited

Tobbe Aug 23, 2022 •

edited

dac09 Aug 23, 2022

dac09 Aug 22, 2022 •

edited

Tobbe Aug 24, 2022

dac09 Aug 22, 2022

jtoar Aug 26, 2022

jtoar Aug 26, 2022

dac09 Aug 29, 2022 •

edited

Tobbe Aug 24, 2022

Tobbe Aug 24, 2022

dac09 Aug 24, 2022

dac09 Aug 24, 2022

Tobbe Aug 24, 2022

dac09 Aug 24, 2022 •

edited

jtoar commented Aug 26, 2022

orta Aug 30, 2022

dac09 Aug 30, 2022

feat(ts): Improves return types of QueryResolvers, MutationResolvers and <Model>Resolvers #6228

feat(ts): Improves return types of QueryResolvers, MutationResolvers and <Model>Resolvers #6228

Conversation

dac09 commented Aug 18, 2022 • edited by Tobbe

What does this do?

nx-cloud bot commented Aug 18, 2022 • edited

☁️ Nx Cloud Report

netlify bot commented Aug 18, 2022 • edited

✅ Deploy Preview for redwoodjs-docs ready!

dac09 Aug 22, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dac09 Aug 22, 2022 • edited

Choose a reason for hiding this comment

Tobbe Aug 23, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dac09 Aug 22, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dac09 Aug 29, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dac09 Aug 24, 2022 • edited

Choose a reason for hiding this comment

jtoar commented Aug 26, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dac09 commented Aug 18, 2022 •

edited by Tobbe

nx-cloud bot commented Aug 18, 2022 •

edited

netlify bot commented Aug 18, 2022 •

edited

dac09 Aug 22, 2022 •

edited

dac09 Aug 22, 2022 •

edited

Tobbe Aug 23, 2022 •

edited

dac09 Aug 22, 2022 •

edited

dac09 Aug 29, 2022 •

edited

dac09 Aug 24, 2022 •

edited