feathersjs / feathers

The API and real-time application framework
https://feathersjs.com
MIT License
15.07k stars 752 forks source link

Resolve* methods should ignore resolver types #2981

Closed engineertdog closed 1 year ago

engineertdog commented 1 year ago

Steps to reproduce

Given a base schema and an extended schema:

export const baseSchema = Type.Object(
  {
    _id: Type.String(),
    createdAt: Type.String(),
    createdBy: Type.String(),
    updatedAt: Type.String(),
    updatedBy: Type.String()
  },
  { $id: "BaseSchema", additionalProperties: false, required: ["_id"] }
);

export const domainSchema = Type.Object(
  {
    ...baseRecordSchema.properties,
    domain: Type.String(),
    description: Type.Optional(Type.String())
  },
  {
    $id: "Domain",
    additionalProperties: false
  }
);

I would expect to have data resolvers and query resolvers for the base model and the extended model. Example:

export const baseRecordCreateDataResolver = resolve<BaseRecord, HookContext>({
  properties: {
    _id: resolveObjectId as any,
    createdBy: resolveObjectId as any,
    createdAt: async (_value, record, context) => {
      if (context.method === "create") {
        return new Date().toISOString();
      }
    },
    updatedBy: resolveObjectId as any,
    updatedAt: async () => {
      return new Date().toISOString();
    }
  }
});

export const baseRecordUpdateDataResolver = resolve<BaseRecord, HookContext>({
  properties: {
    updatedBy: resolveObjectId as any,
    updatedAt: async () => {
      return new Date().toISOString();
    }
  }
});

export const domainDataResolver = resolve<Domain, HookContext>({
  properties: {}
});

Expected behavior

When you call a function such as:

with multiple resolvers, that may utilize different schemas, you shouldn't receive type errors. The resolve* methods should accept the resolvers of different types. Now, this could possibly be problematic in some scenarios. The areas of benefit I could see are extending models. For example, you have a base model that the other models extend from. In that case, you may want to have resolvers for the base model and the extended model to both run for a given service.

Actual behavior

When you use resolvers with different schemas in a resolve* method, you get a type error because the two or more resolvers do not share the same underlying schema.

schemaHooks.resolveData(
          baseRecordCreateDataResolver,
          baseRecordUpdateDataResolver
          domainDataResolver
 )

image

System configuration

Tell us about the applicable parts of your setup.

Module versions (especially the part that's not working):

    "@feathersjs/authentication": "^5.0.0-pre.34",
    "@feathersjs/authentication-local": "^5.0.0-pre.34",
    "@feathersjs/configuration": "^5.0.0-pre.34",
    "@feathersjs/errors": "^5.0.0-pre.34",
    "@feathersjs/feathers": "^5.0.0-pre.34",
    "@feathersjs/koa": "^5.0.0-pre.34",
    "@feathersjs/mongodb": "^5.0.0-pre.34",
    "@feathersjs/schema": "^5.0.0-pre.34",
    "@feathersjs/socketio": "^5.0.0-pre.34",
    "@feathersjs/transport-commons": "^5.0.0-pre.34",
    "@feathersjs/typebox": "^5.0.0-pre.34",

NodeJS version: 18.7.0

Operating System: Windows 11

Browser Version: Firefox 100.xx

React Native Version: N/A

Module Loader: N/A