fix(core): Improve service option usage and method option typings (#2902

)

packages/authentication-oauth/src/index.ts

@@ -4,7 +4,7 @@ import { resolveDispatch } from '@feathersjs/schema'
 
 import { OAuthStrategy, OAuthProfile } from './strategy'
 import { redirectHook, OAuthService } from './service'
-import { getGrantConfig, getServiceOptions, OauthSetupSettings } from './utils'
+import { getGrantConfig, authenticationServiceOptions, OauthSetupSettings } from './utils'
 
 const debug = createDebug('@feathersjs/authentication-oauth')
 
@@ -32,7 +32,7 @@ export const oauth =
     }
 
     const grantConfig = getGrantConfig(authService)
-    const serviceOptions = getServiceOptions(authService, oauthOptions)
+    const serviceOptions = authenticationServiceOptions(authService, oauthOptions)
     const servicePath = `${grantConfig.defaults.prefix || 'oauth'}/:provider`
 
     app.use(servicePath, new OAuthService(authService, oauthOptions), serviceOptions)

packages/authentication-oauth/src/utils.ts

@@ -89,7 +89,7 @@ export const setKoaParams: Middleware = async (ctx, next) => {
   await next()
 }
 
-export const getServiceOptions = (
+export const authenticationServiceOptions = (
   service: AuthenticationService,
   settings: OauthSetupSettings
 ): ServiceOptions => {

packages/feathers/src/application.ts

@@ -145,7 +145,7 @@ export class Feathers<Services, Settings>
   use<L extends keyof Services & string>(
     path: L,
     service: keyof any extends keyof Services ? ServiceInterface | Application : Services[L],
-    options?: ServiceOptions
+    options?: ServiceOptions<keyof any extends keyof Services ? string : keyof Services[L]>
   ): this {
     if (typeof path !== 'string') {
       throw new Error(`'${path}' is not a valid service path.`)
@@ -163,7 +163,7 @@ export class Feathers<Services, Settings>
       return this
     }
 
-    const protoService = wrapService(location, service, options)
+    const protoService = wrapService(location, service, options as ServiceOptions)
     const serviceOptions = getServiceOptions(protoService)
 
     for (const name of protectedMethods) {

packages/feathers/src/declarations.ts

@@ -19,10 +19,23 @@ export interface Paginated<T> {
 /**
  * Options that can be passed when registering a service via `app.use(name, service, options)`
  */
-export interface ServiceOptions {
+export interface ServiceOptions<MethodTypes = string> {
+  /**
+   * A list of custom events that this service emits to clients
+   */
   events?: string[] | readonly string[]
-  methods?: string[] | readonly string[]
+  /**
+   * A list of service methods that should be available __externally__ to clients
+   */
+  methods?: MethodTypes[] | readonly MethodTypes[]
+  /**
+   * Provide a full list of events that this service should emit to clients.
+   * Unlike the `events` option, this will not be merged with the default events.
+   */
   serviceEvents?: string[] | readonly string[]
+  /**
+   * Initial data to always add as route params to this service.
+   */
   routeParams?: { [key: string]: any }
 }
 
@@ -226,7 +239,7 @@ export interface FeathersApplication<Services = any, Settings = any> {
   use<L extends keyof Services & string>(
     path: L,
     service: keyof any extends keyof Services ? ServiceInterface | Application : Services[L],
-    options?: ServiceOptions
+    options?: ServiceOptions<keyof any extends keyof Services ? string : keyof Services[L]>
   ): this
 
   /**

packages/feathers/src/service.ts

@@ -21,6 +21,8 @@ export const defaultEventMap = {
   remove: 'removed'
 }
 
+export const defaultServiceEvents = Object.values(defaultEventMap)
+
 export const protectedMethods = Object.keys(Object.prototype)
   .concat(Object.keys(EventEmitter.prototype))
   .concat(['all', 'around', 'before', 'after', 'error', 'hooks', 'setup', 'teardown', 'publish'])
@@ -33,18 +35,16 @@ export function getHookMethods(service: any, options: ServiceOptions) {
     .concat(methods)
 }
 
-export function getServiceOptions(service: any, options: ServiceOptions = {}): ServiceOptions {
-  const existingOptions = service[SERVICE]
-
-  if (existingOptions) {
-    return existingOptions
-  }
+export function getServiceOptions(service: any): ServiceOptions {
+  return service[SERVICE]
+}
 
+export const normalizeServiceOptions = (service: any, options: ServiceOptions = {}): ServiceOptions => {
   const {
     methods = defaultServiceMethods.filter((method) => typeof service[method] === 'function'),
     events = service.events || []
   } = options
-  const { serviceEvents = Object.values(defaultEventMap).concat(events) } = options
+  const serviceEvents = options.serviceEvents || defaultServiceEvents.concat(events)
 
   return {
     ...options,
@@ -61,7 +61,7 @@ export function wrapService(location: string, service: any, options: ServiceOpti
   }
 
   const protoService = Object.create(service)
-  const serviceOptions = getServiceOptions(service, options)
+  const serviceOptions = normalizeServiceOptions(service, options)
 
   if (Object.keys(serviceOptions.methods).length === 0 && typeof service.setup !== 'function') {
     throw new Error(`Invalid service object passed for path \`${location}\``)

packages/feathers/test/application.test.ts

@@ -441,7 +441,7 @@ describe('Feathers application', () => {
       app.mixins.push(function (service: any, location: any, options: any) {
         assert.ok(service.dummy)
         assert.strictEqual(location, 'dummy')
-        assert.deepStrictEqual(options, getServiceOptions(new Dummy()))
+        assert.deepStrictEqual(options, getServiceOptions(service))
         mixinRan = true
       })
 
@@ -461,7 +461,7 @@ describe('Feathers application', () => {
       app.mixins.push(function (service: any, location: any, options: any) {
         assert.ok(service.dummy)
         assert.strictEqual(location, 'dummy')
-        assert.deepStrictEqual(options, getServiceOptions(new Dummy(), opts))
+        assert.deepStrictEqual(options, getServiceOptions(service))
         mixinRan = true
       })
 

packages/feathers/test/declarations.test.ts

@@ -70,7 +70,9 @@ describe('Feathers typings', () => {
     const app2 = feathers<Record<string, unknown>, Configuration>()
 
     app.set('port', 80)
-    app.use('todos', new TodoService())
+    app.use('todos', new TodoService(), {
+      methods: ['find', 'create']
+    })
     app.use('v2', app2)
 
     const service = app.service('todos')
@@ -84,8 +86,9 @@ describe('Feathers typings', () => {
         all: [],
         create: [
           async (context) => {
-            const { result, data } = context
+            const { result, data, service } = context
 
+            assert.ok(service instanceof TodoService)
             assert.ok(result)
             assert.ok(data)
             assert.ok(context.app.service('todos'))

packages/schema/test/fixture.ts

@@ -1,9 +1,4 @@
-import {
-  feathers,
-  HookContext,
-  Application as FeathersApplication,
-  defaultServiceMethods
-} from '@feathersjs/feathers'
+import { feathers, HookContext, Application as FeathersApplication } from '@feathersjs/feathers'
 import { memory, MemoryService } from '@feathersjs/memory'
 import { GeneralError } from '@feathersjs/errors'
 
@@ -193,7 +188,7 @@ app.use(
   })
 )
 app.use('messages', new MessageService(), {
-  methods: [...defaultServiceMethods, 'customMethod']
+  methods: ['find', 'get', 'create', 'update', 'patch', 'remove', 'customMethod']
 })
 app.use('paginatedMessages', memory({ paginate: { default: 10 } }))
 

packages/transport-commons/src/channels/mixins.ts

@@ -1,5 +1,5 @@
 /* eslint-disable @typescript-eslint/no-unnecessary-type-assertion */
-import { Application, HookContext, getServiceOptions } from '@feathersjs/feathers'
+import { Application, HookContext, getServiceOptions, defaultServiceEvents } from '@feathersjs/feathers'
 import { createDebug } from '@feathersjs/commons'
 import { Channel } from './channel/base'
 import { CombinedChannel } from './channel/combined'
@@ -90,7 +90,7 @@ export function publishMixin() {
         event = ALL_EVENTS
       }
 
-      const { serviceEvents } = getServiceOptions(this)
+      const { serviceEvents = defaultServiceEvents } = getServiceOptions(this) || {}
 
       if (event !== ALL_EVENTS && !serviceEvents.includes(event)) {
         throw new Error(`'${event.toString()}' is not a valid service event`)