VulcanJS / vulcan-npm

The full-stack JavaScript App Framework
https://vulcan-docs.vercel.app
MIT License
30 stars 8 forks source link

Extend a model from another #133

Closed eric-burel closed 1 year ago

eric-burel commented 1 year ago

Is your feature request related to a problem? Please describe.

GraphQL types are often interrelated:

Describe the solution you'd like

1) An easy way to define virtual relations, when the foreign key is in the related model 2) An easy way to extend a model from another in GraphQL. The issue here is that the "Response" model will have to know about "NormalizedResponse" when defining this new virtual relation. But maybe NormalizedResponse is an admin only concept, that should not be known by the "Response" model (this scenario happens in the State of survey form).

Simplified ideal syntax:

// Lives in a  "core" module. I don't want this model to be aware of the existence of an admin area, to reduce dependencies
const ResponseModel = makeModel( {
  schema: { _id: String },
})

// Lives in an "admin" module. We need to extend the "Response" model with a new virtual fields to make display easier.
const NormalizedResponseModel = makeModel( {
  schema: { 
    _id: String, 
   responseId: { type: "relation", kind: "hasOne", model: ResponseModel }
  },
 // The dependency is cleaner: NormalizedResponse extends Response with new virtual fields
 // but the Response model doesn't have to be aware of that
  schemaExtensions: [{
    model: ResponseModel, fieldName: "normalizedResponse", 
   type: "relation", kind: "hasOne", foreignKey: "responseId" 
  }]
})

Describe alternatives you've considered

With extend

See https://stackoverflow.com/questions/48917863/how-do-you-extend-types-in-graphql

The "extend" keyword is not well documented but exists in GraphQL and should work when merging schema.

During schema generation

Since we have control over the graphql schema generation, we could inject extended fields into the existing schema. The difficulty is that currently schema generation is done per-model (that's why Response currently has to know about NormalizedResponse explicitely), we cannot alter one model based on another.

Maybe it would require a second step that generates the "extend" type defintions but merge them immediately after that, so that the final Vulcan schema do not need to actually use "extend" (already merged).

If the "extend" approach doesn't work, we can instead use the intermediate representations of the model (the list of fields parsed from the schema that will be turned into graphql type defintiions) and inject new fields in it from the other model.

Palliative solution

We can define the "extend" clause in a custom type definitions, basically bypassing Vulcan and using the graphql setup more directly. This makes virtual fields slightly harder to follow because they are not part of the model.

eric-burel commented 1 year ago

Current approach defining a schema extension from the extender, and importing it in the extended schema:

/**
 * TODO: currently we have to import this in the "Response" model
 * In the future this should be instead tied to the "NormalizedResponse" model
 * To avoid a dependency Response->NormalizedResponse (we only want NormalizedResponse->Response)
 * @see https://github.com/VulcanJS/vulcan-npm/issues/133
 */
export const responseSchemaExtension = {
  /**
   * TODO: this creates a strong dependency to the admin area
   * And this field is only used in admin display
   *
   * Possible solutions:
   * - put the NormalizedResponse model and Response model into a shared folder
   * - find a way to extend the Response model based on NormalizedResponse
   *
   * + we could create a relation resolver for this in Vulcan for those
   * kind of "virtual" relations (the foreign key is in the related model instead of the current model)
   *
   * @see https://github.com/VulcanJS/vulcan-npm/issues/133
   *
   */
  normalizedResponse: {
    type: Object,
    typeName: "JSON",
    blackbox: true,
    optional: true,
    canRead: ["owners"],
    resolveAs: {
      fieldName: "normalizedResponse",
      typeName: "JSON",
      // TODO: use a relation instead
      resolver: async (response: ResponseDocument) => {
        return await NormalizedResponseConnector.findOne({
          responseId: response._id,
        });
      },
    },
  },
};
eric-burel commented 1 year ago

Better: "belongsToOne" and "belongsToMany" as new relation types

eric-burel commented 1 year ago

Done see https://vulcan-docs.vercel.app/docs/vulcan-fire/customFieldResolvers#method-2---assess-if-a-reversed-relation-is-enough-for-you-advanced

Field is named "reversedRelation" (because there can already be a relation) and types are "hasOneReversed" and "hasManyReversed". This might be slightly complex to understand, but the way it works become intuitive if you have a separate admin area for instance.