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

feat: add default resolver for relation fields #25

Merged
merged 13 commits into from
May 7, 2021
Merged

feat: add default resolver for relation fields #25

merged 13 commits into from
May 7, 2021

Conversation

lvauvillier
Copy link
Contributor

@lvauvillier lvauvillier commented Apr 22, 2021

This is a proposal for default resolver generation:

A new resolvefield is generated in models:

objectType({
  name: User.$name,
  definition(t) {
    t.field(User.posts.name, {
      type: User.posts.type
      resolve: User.posts.resolve
    });
  }
});

or

objectType({
  name: User.$name,
  definition(t) {
    t.field(User.posts.name, User.posts);
  }
});

Need guidance

Current implementation expect to find a prisma instance in context.prisma.
How can we make it configurable?

The old spec (https://gist.github.com/jasonkuhrt/fb5dcb58ba9bf68123460138cb17bc40) suggest to create a new instance and share a singleton across all models, but this is not ideal as a new instance will create a new connexion pool to the db.

Tests are currently broken... I'm waiting for a resolution to start adding tests for this PR

Todo

  • Export default resolver
  • Customize prisma instance location in context
  • Add tests

@lvauvillier lvauvillier changed the title feat: add default resolver for relation fields [WIP] feat: add default resolver for relation fields Apr 22, 2021
@jasonkuhrt
Copy link
Contributor

In your example:

t.field(User.posts.name, Model.posts);

I think you meant User.posts?


How can we make it configurable?

Let's start by making it not configurable. It is, in many cases, not hard for a developer to add Prisma client to another property on context.

I want features to be shipping in very small increments right now.

I suggest we hardcode looking for .prisma on the context.


Later I think we can also consider some singleton settings system.

import { $settings } from 'nexus-prisma'

$settings({
  client: // user passes reference to prisma client instance here
})

Alt API:

import { $set } from 'nexus-prisma'

$set.client = // setter, user passes reference to prisma client instance here

src/generator/helpers/constraints.ts Show resolved Hide resolved
src/generator/models/declaration.ts Outdated Show resolved Hide resolved
src/generator/models/javascript.ts Outdated Show resolved Hide resolved
src/generator/models/javascript.ts Outdated Show resolved Hide resolved
src/generator/models/javascript.ts Outdated Show resolved Hide resolved
@jasonkuhrt
Copy link
Contributor

Dropped some feedback, didn't have time to go deep on the heart of the implementation, just superficial pass.

}

return ctx.prisma[lowerFirst(model.name)]
.findUnique({
Copy link
Contributor

Choose a reason for hiding this comment

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

In my spec about relation resolvers https://gist.github.com/jasonkuhrt/fb5dcb58ba9bf68123460138cb17bc40#relationship-resolver I had imagined trying to use findMany with where as a general solution for all cases.

I'm ok shipping another solution for now if you're doing it, but just wanted to point this out at least.

The thing is, findUnique will not account for all relation cases.

But findMany with where could.

I realize findUnique might be more optimal, but I kind of want/wish to just punt that to Prisma lol, and let the "compiler" figure it out such that it optimizes to a findUnique automatically :D

I see NP usingfindUnique as an optimization. I'm not sure it makes a good starting place. If it is easier for you though, and makes the difference between shipping or not shipping, then ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This solution comes from the old plugin too, i'm not sure if this is the more efficient but it solves all relation types without adding specific case to get the first item.

Copy link
Contributor

@jasonkuhrt jasonkuhrt Apr 23, 2021

Choose a reason for hiding this comment

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

How does findUnique work with lists e.g. 1:n relations?

Or is this PR only tackling non-list relations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here findUnique always retrieve the current root object. Then we let prisma client automatically retrieve related objects using the appropriate prisma client method.
So, Prisma client handle magically for us both 1-1 and 1-n :)

This is the logic implemented in the old plugin

@lvauvillier lvauvillier changed the title [WIP] feat: add default resolver for relation fields feat: add default resolver for relation fields Apr 23, 2021
Copy link
Contributor

@jasonkuhrt jasonkuhrt left a comment

Choose a reason for hiding this comment

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

Ok this seems like a great start, it has been open long enough, let's keep things moving. Thanks @lvauvillier!

@jasonkuhrt
Copy link
Contributor

jasonkuhrt commented Apr 30, 2021

@lvauvillier looks like tests are failing on CI, please fix that first 🙏

@lvauvillier
Copy link
Contributor Author

I dont know how to solve the last CI issues:
e2e test never pass on my mac. When npm installs dependencies, they are not all present in the node_module of the generated project.

I also have no idea how to fix the eslint warnings. As we discussed, adding types to default resolver args is hard to solve...

@codecov-commenter
Copy link

Codecov Report

Merging #25 (b4985bb) into main (624c745) will decrease coverage by 0.02%.
The diff coverage is 15.09%.

@@            Coverage Diff             @@
##             main      #25      +/-   ##
==========================================
- Coverage   32.17%   32.15%   -0.03%     
==========================================
  Files          23       24       +1     
  Lines         317      367      +50     
  Branches       59       70      +11     
==========================================
+ Hits          102      118      +16     
- Misses        208      242      +34     
  Partials        7        7              

@jasonkuhrt
Copy link
Contributor

Thanks @lvauvillier!

@jasonkuhrt jasonkuhrt merged commit 4f5cd70 into graphql-nexus:main May 7, 2021
github-actions bot pushed a commit that referenced this pull request Jan 7, 2023
## 1.0.0 (2023-01-07)

### ⚠ BREAKING CHANGES

* **deps:** update dependency graphql to v16

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Rostislav Simonik <rostislav.simonik@technologystudio.sk>
* scalars module only exports named exports (#187)
* support latest Prisma (#164)
* no types namespace (#130)
* remove support for prisma 2.17
* support prisma 2.30 (#114)
* Prisma Int @id maps to GraphQL Int  (#23)
* better names, auto-import friendly, jsdoc (#12)
* validate that peer dep requirements are met
* **deps:** nexua and @prisma/client are peer dependencies
* hello world

### Features

* add bytes scalar support ([#75](#75)) ([88fd092](88fd092))
* add default resolver for relation fields ([#25](#25)) ([4f5cd70](4f5cd70))
* add nexus_prisma bin for =<2.17 prisma users ([469c9e9](469c9e9))
* add nexusPrisma default and named export ([704f07c](704f07c))
* add peer dep support for prisma 2.18.x ([2e898ab](2e898ab))
* add support for BigInt scalar ([#56](#56)) ([67ce824](67ce824))
* add support for Decimal scalar ([#96](#96)) ([74da7c2](74da7c2))
* add support for reading configuration at generation time ([#27](#27)) ([275e03f](275e03f))
* control rejectOnNotFound client setting ([#135](#135)) ([01daf38](01daf38))
* **deps:** nexua and @prisma/client are peer dependencies ([b28217b](b28217b))
* emit model scalar fields ([#5](#5)) ([3a0a75a](3a0a75a))
* esm support ([#127](#127)) ([eec4932](eec4932))
* gentime setting for output directory ([#166](#166)) ([83889bf](83889bf))
* hello world ([9667cd0](9667cd0))
* Json & DateTime custom scalar support ([#9](#9)) ([df51143](df51143)), closes [#8](#8)
* Prisma Int [@id](https://github.com/id) maps to GraphQL Int  ([#23](#23)) ([624c745](624c745))
* settings system for runtime & gentime ([#42](#42)) ([ef76e45](ef76e45))
* **settings:** allow customization of prisma client import ([#49](#49)) ([c8b5f6c](c8b5f6c))
* **settings:** allow disable jsdoc guide ([#136](#136)) ([9418649](9418649))
* support latest Prisma ([#164](#164)) ([594699d](594699d))
* support prisma 2.30 ([#114](#114)) ([b7c7927](b7c7927))
* support prisma up to 2.24 ([#70](#70)) ([2704ff2](2704ff2))
* support prisma up to 3 ([#147](#147)) ([0020742](0020742))
* support projecting enums ([#18](#18)) ([1c1cd13](1c1cd13))
* turn prisma client on graphql context validation into a formal check ([#180](#180)) ([72817f1](72817f1))
* Update official Prisma support up to 2.27 ([#100](#100)) ([db1010d](db1010d))
* Use JSONResolver for Json scalar ([#231](#231)) ([e4143ac](e4143ac))
* validate that peer dep requirements are met ([7976bf5](7976bf5))

### improve

* better names, auto-import friendly, jsdoc ([#12](#12)) ([6e395b9](6e395b9))
* no types namespace ([#130](#130)) ([cbe3df8](cbe3df8))
* remove support for prisma 2.17 ([e007721](e007721))

### Refactoring

* peer dep failure state labels ([03d3c64](03d3c64))
* resolver "constraints" ([#165](#165)) ([3c9991b](3c9991b))
* **tests:** use kont for tests ([#149](#149)) ([1218c7e](1218c7e))

### Documentation

* "which should I use" guide ([599b76d](599b76d))
* add jsdoc for $settings ([#90](#90)) ([98a2267](98a2267))
* add missing issue reference ([eefa884](eefa884))
* architecture diagram ([5f4970f](5f4970f)), closes [#7](#7)
* cover projecting 1-to-1 relations ([#32](#32)) ([bb70ea4](bb70ea4))
* **jsdoc:** docPropagation docs ([#55](#55)) ([5b4e5e6](5b4e5e6))
* list enum members in jsdoc ([#92](#92)) ([d806d56](d806d56))
* mention patch ver support policy ([8e61651](8e61651))
* peer deps validation ([4992741](4992741)), closes [#2](#2)
* **readme:** adjust readiness disclaimer ([0e94cea](0e94cea))
* **readme:** remove now-resolved limitation caveat ([b5fed98](b5fed98))
* setup hello world nextra site ([784705c](784705c))
* update to nextra 2.0.0 ([#232](#232)) ([fccffd5](fccffd5))
* update to nexus@^1.1 api ([#95](#95)) ([1d31a76](1d31a76))
* **website:** fix links in docs ([#144](#144)) ([abd56f2](abd56f2))

### Testing

* **e2e:** adjust kitchen-sink snapshot ([#234](#234)) ([f831369](f831369))
* **e2e:** adjust ts-node-import-error snapshot ([#230](#230)) ([e464f2f](e464f2f))
* extract hardcoded package.json declarations to fixtures ([#309](#309)) ([8cebf8a](8cebf8a))

### chore

* **deps:** update dependency graphql to v16 ([#256](#256)) ([b6678a4](b6678a4))

### Bug fixes

* add lodash as production dep ([627aa54](627aa54)), closes [#107](#107)
* bring back support for jest ([1705a54](1705a54)), closes [#137](#137)
* **deps:** update dependency @reach/skip-nav to v0.18.0 ([#300](#300)) ([1ece744](1ece744))
* **deps:** update dependency @types/node to v18.11.17 ([#324](#324)) ([86a6c4d](86a6c4d))
* **deps:** update dependency debug to ^4.3.4 ([#289](#289)) ([53a5b47](53a5b47))
* **deps:** update dependency decimal.js to ^10.4.2 ([#301](#301)) ([38d02f6](38d02f6))
* **deps:** update dependency decimal.js to ^10.4.3 ([#325](#325)) ([21cea98](21cea98))
* **deps:** update dependency dotenv to ^9.0.2 ([#326](#326)) ([fca26b0](fca26b0))
* **deps:** update dependency fs-jetpack to v5 ([#255](#255)) ([7fdb71c](7fdb71c))
* **deps:** update dependency graphql-scalars to ^1.20.1 ([#302](#302)) ([38d47d6](38d47d6))
* **deps:** update dependency kleur to ^4.1.5 ([#290](#290)) ([3c2310f](3c2310f))
* **deps:** update dependency nextra to v2.0.0-beta.41 ([#273](#273)) ([a523891](a523891))
* **deps:** update dependency nextra-theme-docs to v2.0.0-beta.41 ([#274](#274)) ([73f99bf](73f99bf))
* **deps:** update dependency semver to ^7.3.8 ([#291](#291)) ([1cbfb25](1cbfb25))
* **deps:** update dependency tslib to ^2.4.1 ([#303](#303)) ([ec603b0](ec603b0))
* **deps:** update dependency typescript to v4.9.4 ([#251](#251)) ([bb3d9d7](bb3d9d7))
* **deps:** update nextra packages to v2.0.1 ([#316](#316)) ([f3923e2](f3923e2))
* **deps:** update nextra packages to v2.0.2 ([#346](#346)) ([ee97081](ee97081))
* **deps:** update nextra packages to v2.0.3 ([#349](#349)) ([84170d1](84170d1))
* description type should not be null ([#24](#24)) ([cffc94d](cffc94d))
* **docs:** update custom settings example ([#215](#215)) ([8b56ec9](8b56ec9))
* endent via dedent introduces unexpected newlines on Windows ([#51](#51)) ([2447f56](2447f56))
* graphql peerDependency ([#233](#233)) ([0086763](0086763))
* grpahql and floggy dependencies ([#188](#188)) ([774624f](774624f))
* handle multiline prisma docs ([#134](#134)) ([f9e2f2e](f9e2f2e))
* import path on windows ([#145](#145)) ([4699b90](4699b90))
* import prisma client for instanceof check using configured path ([#62](#62)) ([b796689](b796689))
* list def typing ([#77](#77)) ([72cc944](72cc944))
* module `fs-jetpack` not found ([#11](#11)) ([4f83b26](4f83b26))
* ncc support ([#113](#113)) ([9c7e552](9c7e552))
* output mjs files for ES modules support ([#192](#192)) ([cf59aae](cf59aae))
* remove bad prisma client on ctx check & export $settings ([#60](#60)) ([60a77cd](60a77cd))
* remove colors from the the result to fix the local test ([#225](#225)) ([684fa20](684fa20))
* remove lingering console.log ([d16f763](d16f763))
* resolve path ~ when checking if can import at @prisma/client ([#104](#104)) ([8eee072](8eee072))
* scalars module only exports named exports ([#187](#187)) ([5223f9e](5223f9e))
* setup nodejs 14 for publishing job on ci ([4cbb0de](4cbb0de))
* typegen guards for undefined relations ([#126](#126)) ([a27fc1a](a27fc1a))
* update prisma client dep to 3 ([#148](#148)) ([fa58349](fa58349))
* use import id @prisma/client by default when possible ([#88](#88)) ([5599a65](5599a65))

### CI

* add pull request workflow documentation build commit status check ([#314](#314)) ([4522e1d](4522e1d))
* configure renovate config to update only the lock file for prisma packages ([#344](#344)) ([3560a41](3560a41))
* configure renovate to automerge minor and patch chore(deps) ([#284](#284)) ([2022809](2022809))
* extract tests into reusable workflow and refactor releases ([#345](#345)) ([27e99a8](27e99a8))
* fix package version ([#317](#317)) ([4e02312](4e02312))
* group nextra packages for renovate upgrades ([#315](#315)) ([21471e8](21471e8))
* ignore execa from renovate updates due exclusive esm support ([#337](#337)) ([99110c8](99110c8))
* refactor tests to use local database ([#311](#311)) ([7559995](7559995))
* reflect local database for tests in trunk github actions workflow ([#312](#312)) ([8bfd561](8bfd561))
* switch dependencies into dev dependencies in tests ([#319](#319)) ([11ee750](11ee750))
* unify trunk and pull request github actions workflow ([#313](#313)) ([6a73b25](6a73b25))
* update renovate config to use version range to ignore packages migrated to pure esm ([#343](#343)) ([23ed616](23ed616))
@rostislav-simonik-nexus-prisma-admin
Copy link
Collaborator

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants