ssalbdivad
commented
4 years ago @jasonkuhrt @Weakky There's a lot here, so to make sure I have enough time to finish up stage 1, I'm just going to list my first couple thoughts:
-
This comment referenced the
create/connect(both disabled by default, implies exclusivity) approach we had discussed, while I notice above we've reverted tocreateable/connectable(both enabled by default). Was this intentional?
queryType({ definition(t) { t.crud.bars({ inputs: { age: { ordering: true }, role: { ordering: true }, ... } }) } })```
I agree this API (with everything organized by field) is more intuitive despite the differences between where the input options manifest themselves.
3.
queryType({ definition(t) { t.crud.bars({ $pagination: true, $ordering: { age: true, role: true }, ... }) } })
I like the idea of removing the inputs level. Since it seems like this will be a breaking change anyways, what if a crud function took up to two parameters, with one representing field configs and the other representing resolver configs:
queryType({
definition(t) {
t.crud.bars({
age: {
ordering: true
},
role: {
ordering: true
},
...
}, {alias: "getBars", type: "BarsType"})
}
})This approach looks clean and mirrors gql structure. If a single object is passed, we could use its shape to determine whether its an input config or a resolver config.
jasonkuhrt
Weakky
maartenraes
RafaelKr
Akxe
durdenx
lvauvillier
Original Description
Show
This proposal comes on the heels of original work by @ssalbdivad on #540 and #566, and by recent discussion between @Weakky and @jasonkuhrt. ## Overview ### Refresh on Computed Inputs #540 made it possible for users to express that certain CRUD inputs should not be part of the schema and should instead have their value provided by the given function. ##### Example before: ```gql mutation { createUser(data: { createdAt: ..., ... }) { ... } } ``` given/then: ```ts mutationType({ definition(t) { t.crud.createUser({ computedInputs: { createdAt: (_root, args, ctx) => ... } }) } }) ``` ```gql mutation { # v--- createdAt no longer here createOne(data: { ... }) { ... } } ``` ### Refresh on "Upfilter Key" #566 has been working toward making it possible to control the `connect` `create` aspects of CRUD inputs on relational data (the original feature idea started out even more general but focused since toward this). The gist is as follows: ##### Example before: ```gql mutation { createUser(data: { mother: { connectable: { id: ... } }, ... }) { ... } } # or mutation { createUser(data: { mother: { creatable: { email: ... } }, ... }) { ... } } # or mutation { createUser(data: { mother: { connectable: { ... }, creatable: { ... } }, ... }) { ... } } ``` given/then: ```ts mutationType({ definition(t) { t.crud.createUser({ mother: { connectable: true } }) } }) ``` ```gql mutation onlyConnect { createUser(data: { mother: { connectable: { id: ... } }, ... }) { ... } } ``` given/then: ```ts mutationType({ definition(t) { t.crud.createUser({ mother: { creatable: true } }) } }) ``` ```gql mutation onlyCreate { createUser(data: { mother: { creatable: { id: ... } }, ... }) { ... } } ``` ## Unifying "Upfilter Key" and Computed Inputs For what its worth, you can view raw code notes by @Weakky and @jasonkuhrt here. It is like raw data, not really consumable information. Again for what its worth.LiveShare "Code Notes"
```js // // overview // // Layer 1 - global (lowest precedence) nexusPrisma({ crud?: { relations?: { create?: boolean, // default false connect?: boolean, // default false } } }) // nexusPrisma({ // crud?: { // relations?: { // create?: true, // connect?: true, // } // } // }) // Layer 2 - model (spans all CRUD) objectType({ type: 'Toto', crud: { create?: boolean, connect?: boolean, }, definition(t) { // ... } }) // // Layer 2.5 - model field (spans all CRUD) ??????????????????????? // objectType({ // type: 'Foo', // definition(t) { // t.model.toto({ // create?: boolean, // <== creatable: false (not possible) if toto is a required field // connect?: boolean, // }) // } // }) // Layer 3 - CRUD input field (highest precedence) t.model.$all() objectType({ definition(t) { t.model.foo({ filtering: boolean, ordering: boolean, pagination: boolean }) } }) queryType({ definition(t) { t.crud.foo({ pagination: boolean, ordering: boolean | { toto: true } }) } }) queryType({ definition(t) { t.crud.foo({ where: true }) t.crud.foo({ toto: { ordering: false }, field: { ordering: false }, field2: { ordering: false } }) } }) ` mutation { createFoo({ data: { toto: { connectable: { id: "" } }, } }) } ` // --------------------------------------------------- objectType({ name: 'Mother', crud: { creatable: true // <== typegen, changes types in the lower layers }, definition(t) {...} }) type Toto = { mother?: string, [x: string]: any } mutationType({ definition(t) { t.crud.createUser({ inputs: { /// VVVVVVVVV mother: { // <<<<<<<-- dynamic key from PSL /// ^^^^^^ connectable: { }, // Warning: this doesn't make sense }, }, customResolver: ({ root, args, ctx, originalResolver }) => { }, alias: {} }) } }) // gives: ` mutation { createFoo({ data: { age: 11 } }) } ` // --------------------------------------------------- mutationType({ definition(t) { t.crud.createUser({ mother: { computed: () => {}, // doesnt mean anything to use create|connect creatable: { age: { computed: () => 42 } }, connectable: { computed: () => {} } }, }) } }) ` mutation { createFoo({ data: { mother: { creatable: { name: "something", }, connectable: { id: "" | email: "" } } } }) } ` // --------------------------------------------------- // Case study of feature overlap // "I want to remove the field" // -> upfilterKey: Remove one field // -> computedInputs: remove one field AND provide a default value // -> Proposal: Allows to remove a field, and optionally, provide a default value if needed (eg: field is required) // base objectType({ type: 'Foo', definition(t) { t.model.toto() } }) // default ` mutation { createFoo({ toto: { creatable: { ... }, connectable: { ... } } }) { } } ` // with computed input queryType({ definition(t) { t.crud.createFoo({ computedInputs: { toto: ({ root, args, ctx }) => ({ creatable: { name: "default_name" } }) }, }) }) ` mutation { createFoo({ ... }) { } } ` // with upfilter objectType({ type: 'Foo', definition(t) { t.model.toto({ create?: false, // <== creatable: false (not possible) if toto is a required field connect?: false }) } }) // --------------------------------------------------- // Case study of runtime error, UNLESS // Temporarily objectType({ type: 'Foo', definition(t) { t.model.toto({ creatable: false, connectable: false }) } }) queryType({ definition(t) { t.createFoo({ computedInputs: { creatable: () => ({ toto: 'default-VALUE' // <== That is needed because toto is required }) } }) } }) // --------------------------------------------------- // Case study of create/connect default = false // so... // IF e.g. Foo.toto is REQUIRED queryType({ definition(t) { t.createFoo() // error } }) // solution 1 queryType({ definition(t) { t.createFoo({ toto: { connectable: true } }) } }) // solution 2 queryType({ definition(t) { t.createFoo({ computedInputs: { toto: () => ... } }) } }) // notes: // capabilities of the field --vs-- parameters to a capability ```We thought of colocating Computed Inputs feature as a "capability" like any other available per field. ```ts mutationType({ definition(t) { t.crud.createUser({ inputs: { createdAt: { computed(_root, args, ctx) { return Date.now() } }, mother: { connectable: true } } }) } }) ``` Because it appears to be invalid to have an input field computed _and_ have capabilities configured we could also move the computed up as a function-alternative to capabilities object config: ```ts mutationType({ definition(t) { t.crud.createUser({ inputs: { createdAt(_root, args, ctx) { return Date.now() }, mother: { connectable: true } } }) } }) ``` This function-or-config approach is what we'll stick with in the following examples. ## Details ### Why nest inside `inputs`? Why not this? ```ts mutationType({ definition(t) { t.crud.createUser({ createdAt(_root, args, ctx) { return Date.now() }, mother: { connectable: true } }) } }) ``` Because there are other settings at root, thus model fields could collide with built in settings. We did however think about a future where we prfix all non-model-field-name keys with `$` so that it 1) we can do this, and 2) it generally becomes very clear what parts of config correspond to the data layer models. Example: ```ts mutationType({ definition(t) { t.crud.createUser({ $alias: 'foobar', $type: 'Soup', createdAt(_root, args, ctx) { return Date.now() }, mother: { connectable: true } }) } }) queryType({ definition(t) { t.crud.bars({ $pagination: true, $ordering: { age: true, role: true }, ... }) } }) ``` This will be discussed as its own issue, later. ### Don't we lose API syymetry by having fields in capabilities with query crud? Aren't the following showing API inconsistence? ```ts mutationType({ definition(t) { t.crud.createUser({ foo(_root, args, ctx) { return Date.now() }, bar: { connectable: true } }) } }) queryType({ definition(t) { t.crud.bars({ ordering: { age: true, role: true }, ... }) } }) ``` Maybe not. Maybe it is the difference between capabilities on a field and params to a capabilitiy... Consider the resulting schema: ```gql mutation { # __ Capability for field `bar` # / V createUser({ data: { bar: { connectable: {...} } } }) { ... } } query { # Capability for field `bars` # / | Params to a capability # / v V bars(orderBy: { role: ASC }) { ... } } ``` But... maybe we're hookwinking ourselves here with elaborate made-up semantics... Maybe the following consistentcy really would be the best direction ultimately: ```ts queryType({ definition(t) { t.crud.bars({ inputs: { age: { ordering: true }, role: { ordering: true }, ... } }) } }) ``` But... going back to the API split again, it is split in a way that most closely resembles _visually_/_literally_ the SDL/input args nesting. This seems to suggest a deeper harmony, a positive sign for the API. ### About onion layers Borrowing from #566, this API would have layers like explored there: ```ts // Layer 1 - global (lowest precedence) nexusPrisma({ crud?: { relations?: { create?: boolean, // default false connect?: boolean, // default false } } }) // Layer 2 - model (spans all CRUD) objectType({ type: 'Toto', crud: { relations?: { create?: boolean, connect?: boolean, } }, definition(t) { // ... } }) // Layer 3 - CRUD input field (highest precedence) mutationType({ definition(t) { t.crud.foo({ inputs: { toto: { create?: boolean, connect?: boolean } } }) } }) ``` ### About Encoding the Cascade The following is harmelss but redundant: ```ts nexusPrisma({ crud?: { relations?: { creatable: true, // default false connectable: true, // default false } } }) objectType({ type: 'Toto', crud: { relations?: { creatable: true, // <-- redundant connectable: true, // <-- redundant } }, definition(t) { // ... } }) ``` Redundancy can be user laziness (changed root, not updating codebase) but also user confusion. We don't want to raise an error and block building for something that wouldn't harm runtime. But, we do think it would be great to encode the feedback as a warning. And if we can encode the warning, then we should be to without much effort introduce a codemode to "autofix" the issue. Maybe this can be part of an ESLint plugin, for example. ```ts objectType({ type: 'Toto', crud: { relations?: { creatable: true, // ^^^^^^^^^^^^^ Redundant because create-relations are enabled by default globally connectable: true, // ^^^^^^^^^^^^^ Redundant because connect-relations are enabled by default globally } }, definition(t) { // ... } }) ``` ### About Overriding
This idea was aborted, left here for posterity
In the following two examples, do you think either will result in `toto` relation being connectable? ```ts nexusPrisma({ crud?: { relations?: { creatable: false, connectable: true, } } }) mutationType({ definition(t) { t.crud.foo({ toto: { creatable: true, } }) } }) ``` ```ts nexusPrisma({ crud?: { relations?: { creatable: true, connectable: true, } } }) mutationType({ definition(t) { t.crud.foo({ toto: { creatable: true, } }) } }) ``` Maybe not...The thinking being 1) WYSIWYG 2) create/connect capability are atomic, once you begin configuring part of them, its as if you're taking all of it on, and it defaults to off (see point 1). This seems controversial though. By not embracing boolean flip-and-cascade through the layers we remove our ability to create the great linter experience above. setting `false` becomes only possible at the root level. And no other capabilities so far have this "atomic" linking––they all appear to be behaviourly independent peers, which this idea would somehow deceptively compromise...Current Spec
comment
Related
95
518
566