feat: Allow hooks on all methods and add externalMethods option (#3457)

packages/feathers/src/declarations.ts

@@ -30,9 +30,16 @@ export interface ServiceOptions<MethodTypes = string> {
    */
   events?: string[] | readonly string[]
   /**
-   * A list of service methods that should be available __externally__ to clients
+   * A list of service methods that `all` hooks will apply to.
+   * Defaults to the standard service methods (find, get, create, update, patch, remove)
+   * that exist on the service.
    */
   methods?: MethodTypes[] | readonly MethodTypes[]
+  /**
+   * A list of service methods that should be available __externally__ to clients
+   * via transports like HTTP or WebSockets. Defaults to `methods` if not specified.
+   */
+  externalMethods?: 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.

packages/feathers/src/hooks.ts

@@ -28,6 +28,11 @@ type HookStore = {
   error: { [method: string]: HookFunction[] }
   collected: { [method: string]: AroundHookFunction[] }
   collectedAll: { before?: AroundHookFunction[]; after?: AroundHookFunction[] }
+  /**
+   * Methods that should receive "all" hooks. Methods not in this set
+   * will only receive their specific hooks, not "all" hooks.
+   */
+  allMethods?: Set<string>
 }
 
 type HookEnabled = { __hooks: HookStore }
@@ -55,15 +60,19 @@ export function convertHookData(input: any) {
   return result
 }
 
-export function collectHooks(target: HookEnabled, method: string) {
-  const { collected, collectedAll, around } = target.__hooks
+export function collectHooks(target: HookEnabled, method: string, includeAll: boolean = true) {
+  const { collected, collectedAll, around, allMethods } = target.__hooks
+
+  // Only include "all" hooks if includeAll is true AND
+  // (allMethods is not defined OR method is in allMethods)
+  const shouldIncludeAll = includeAll && (!allMethods || allMethods.has(method))
 
   return [
-    ...(around.all || []),
+    ...(shouldIncludeAll ? around.all || [] : []),
     ...(around[method] || []),
-    ...(collectedAll.before || []),
+    ...(shouldIncludeAll ? collectedAll.before || [] : []),
     ...(collected[method] || []),
-    ...(collectedAll.after || [])
+    ...(shouldIncludeAll ? collectedAll.after || [] : [])
   ] as AroundHookFunction[]
 }
 
@@ -185,11 +194,11 @@ export function hookMixin<A>(this: A, service: FeathersService<A>, path: string,
 
   const hookMethods = getHookMethods(service, options)
 
-  const serviceMethodHooks = hookMethods.reduce((res, method) => {
+  const createMethodHookManager = (app: A, method: string) => {
     const params = (defaultServiceArguments as any)[method] || ['data', 'params']
 
-    res[method] = new FeathersHookManager<A>(this, method).params(...params).props({
-      app: this,
+    return new FeathersHookManager<A>(app, method).params(...params).props({
+      app,
       path,
       method,
       service,
@@ -203,15 +212,31 @@ export function hookMixin<A>(this: A, service: FeathersService<A>, path: string,
         this.http.status = value
       }
     })
+  }
 
+  const serviceMethodHooks = hookMethods.reduce((res, method) => {
+    res[method] = createMethodHookManager(this, method)
     return res
   }, {} as BaseHookMap)
 
   const registerHooks = enableHooks(service)
 
+  // Set which methods should receive "all" hooks (from the methods option)
+  ;(service as any).__hooks.allMethods = new Set(hookMethods)
+
   hooks(service, serviceMethodHooks)
 
-  service.hooks = function (this: any, hookOptions: any) {
+  service.hooks = createServiceHooksMethod(this, registerHooks, createMethodHookManager)
+
+  return service
+}
+
+function createServiceHooksMethod<A>(
+  app: A,
+  registerHooks: ReturnType<typeof enableHooks>,
+  createMethodHookManager: (app: A, method: string) => FeathersHookManager<A>
+) {
+  return function (this: any, hookOptions: any) {
     if (hookOptions.before || hookOptions.after || hookOptions.error || hookOptions.around) {
       return registerHooks.call(this, hookOptions)
     }
@@ -221,17 +246,27 @@ export function hookMixin<A>(this: A, service: FeathersService<A>, path: string,
     }
 
     Object.keys(hookOptions).forEach((method) => {
-      const manager = getManager(this[method])
+      // Skip 'all' since it's not an actual method
+      if (method === 'all') {
+        return
+      }
 
+      let manager = getManager(this[method])
+
+      // Lazily create hook manager for methods not initially configured
       if (!(manager instanceof FeathersHookManager)) {
-        throw new Error(`Method ${method} is not a Feathers hooks enabled service method`)
+        if (typeof this[method] !== 'function') {
+          throw new Error(`Method ${method} does not exist on this service`)
+        }
+
+        const methodManager = createMethodHookManager(app, method)
+        hooks(this, { [method]: methodManager })
+        manager = methodManager
       }
 
       manager.middleware(hookOptions[method])
     })
 
     return this
   }
-
-  return service
 }

packages/feathers/src/hooks/feathers/hooks.test.ts

@@ -364,6 +364,100 @@ describe('hooks basics', () => {
     )
   })
 
+  it('can register hooks on methods not in the methods option', async () => {
+    class Dummy {
+      async get(id: Id) {
+        return { id }
+      }
+
+      async internalMethod(data: any) {
+        return data
+      }
+    }
+
+    const app = feathers<{
+      dummy: Dummy
+    }>().use('dummy', new Dummy(), {
+      methods: ['get'] // internalMethod not in methods
+    })
+
+    // Should be able to register hooks on internalMethod even though it's not in methods
+    app.service('dummy').hooks({
+      internalMethod: [
+        async (context, next) => {
+          context.data.fromHook = true
+          await next()
+        }
+      ]
+    })
+
+    const result = await app.service('dummy').internalMethod({
+      message: 'testing'
+    })
+
+    assert.deepStrictEqual(result, {
+      message: 'testing',
+      fromHook: true
+    })
+  })
+
+  it('all hooks only apply to methods in the methods option', async () => {
+    const allHookCalls: string[] = []
+
+    class Dummy {
+      async get(id: Id) {
+        return { id }
+      }
+
+      async create(data: any) {
+        return data
+      }
+
+      async helperMethod(data: any) {
+        return data
+      }
+    }
+
+    const app = feathers<{
+      dummy: Dummy
+    }>().use('dummy', new Dummy(), {
+      methods: ['get', 'create'] // helperMethod not in methods
+    })
+
+    // Register an "all" hook
+    app.service('dummy').hooks({
+      around: {
+        all: [
+          async (context, next) => {
+            allHookCalls.push(context.method)
+            await next()
+          }
+        ]
+      }
+    })
+
+    // Register a specific hook on helperMethod
+    app.service('dummy').hooks({
+      helperMethod: [
+        async (context, next) => {
+          context.data.specificHook = true
+          await next()
+        }
+      ]
+    })
+
+    // Call methods
+    await app.service('dummy').get(1)
+    await app.service('dummy').create({ test: true })
+    const helperResult = await app.service('dummy').helperMethod({ test: true })
+
+    // "all" hook should have been called for get and create, but NOT helperMethod
+    assert.deepStrictEqual(allHookCalls, ['get', 'create'])
+
+    // But the specific hook should have run on helperMethod
+    assert.deepStrictEqual(helperResult, { test: true, specificHook: true })
+  })
+
   it('normalizes params to object even when it is falsy (#1001)', async () => {
     const app = feathers().use('/dummy', {
       async get(id: Id, params: Params) {

packages/feathers/src/http/index.test.ts

@@ -1,8 +1,12 @@
 import { beforeAll, describe, it, expect } from 'vitest'
+import { createServer } from 'node:http'
 import { restTests, verify, getApp, createTestServer } from '../../fixtures/index.js'
-import { CORS_HEADERS } from './index.js'
+import { CORS_HEADERS, createHandler } from './index.js'
+import { toNodeHandler } from './node.js'
+import { feathers } from '../index.js'
 
 const TEST_PORT = 4444
+const EXTERNAL_METHODS_PORT = 4445
 
 describe('http test', () => {
   beforeAll(async () => {
@@ -314,4 +318,94 @@ describe('http test', () => {
   })
 
   restTests('http', 'todos', TEST_PORT)
+
+  describe('externalMethods option', () => {
+    let externalMethodsHookCalls: string[] = []
+
+    beforeAll(async () => {
+      class ExternalMethodsService {
+        async find() {
+          return [{ id: 1, name: 'test' }]
+        }
+
+        async get(id: string) {
+          return { id, name: 'test' }
+        }
+
+        async internalMethod(data: any) {
+          return { ...data, processed: true }
+        }
+      }
+
+      const app = feathers<{ external: ExternalMethodsService }>()
+
+      app.use('external', new ExternalMethodsService(), {
+        // methods controls which methods get "all" hooks
+        methods: ['find', 'get', 'internalMethod'],
+        // externalMethods controls which methods are exposed via HTTP
+        externalMethods: ['find', 'get'] // internalMethod NOT exposed
+      })
+
+      // Add an "all" hook to track which methods it runs on
+      app.service('external').hooks({
+        around: {
+          all: [
+            async (context, next) => {
+              externalMethodsHookCalls.push(context.method)
+              await next()
+            }
+          ]
+        }
+      })
+
+      const handler = createHandler(app)
+      const nodeServer = createServer(toNodeHandler(handler))
+
+      await new Promise<void>((resolve) => {
+        nodeServer.listen(EXTERNAL_METHODS_PORT, () => resolve())
+      })
+
+      await app.setup(nodeServer)
+    })
+
+    it('allows access to methods in externalMethods', async () => {
+      externalMethodsHookCalls = []
+
+      const res = await fetch(`http://localhost:${EXTERNAL_METHODS_PORT}/external`)
+      expect(res.status).toBe(200)
+
+      const data = await res.json()
+      expect(data).toEqual([{ id: 1, name: 'test' }])
+
+      // "all" hook should have run
+      expect(externalMethodsHookCalls).toContain('find')
+    })
+
+    it('blocks access to methods not in externalMethods', async () => {
+      const res = await fetch(`http://localhost:${EXTERNAL_METHODS_PORT}/external`, {
+        method: 'POST',
+        headers: {
+          'Content-Type': 'application/json',
+          'X-Service-Method': 'internalMethod'
+        },
+        body: JSON.stringify({ data: 'test' })
+      })
+
+      expect(res.status).toBe(405)
+
+      const error = await res.json()
+      expect(error.name).toBe('MethodNotAllowed')
+      expect(error.message).toBe('Method `internalMethod` is not supported by this endpoint.')
+    })
+
+    it('runs "all" hooks on methods in methods option (including non-external)', async () => {
+      externalMethodsHookCalls = []
+
+      // Call find via HTTP (external)
+      await fetch(`http://localhost:${EXTERNAL_METHODS_PORT}/external`)
+
+      // "all" hook runs on find
+      expect(externalMethodsHookCalls).toEqual(['find'])
+    })
+  })
 })

packages/feathers/src/http/index.ts

@@ -4,7 +4,7 @@ import { BadRequest, MethodNotAllowed, NotFound } from '../errors.js'
 import { hooks, middleware } from '../hooks/index.js'
 import * as utils from './utils.js'
 import { BODY_METHODS, bodyParser, errorHandler, queryParser } from './middleware.js'
-import { createContext, getServiceOptions } from '../index.js'
+import { createContext, getServiceOptions, getExternalMethods } from '../index.js'
 
 export type HttpParams<Q> = Params<Q> & {
   request?: Request
@@ -138,11 +138,11 @@ export function createHandler(
     const methodOverride = headers[utils.METHOD_HEADER]
     // Get the service method for the request.
     const method = utils.getServiceMethod(request.method, id, methodOverride)
-    // Get the methods supported by the service.
-    const { methods } = getServiceOptions(service)
+    // Get the methods exposed externally by the service.
+    const externalMethods = getExternalMethods(getServiceOptions(service))
 
-    // If the service does not support the requested method, throw an error.
-    if (methods && !methods.includes(method)) {
+    // If the service does not support the requested method externally, throw an error.
+    if (externalMethods && !externalMethods.includes(method)) {
       throw new MethodNotAllowed(`Method \`${method}\` is not supported by this endpoint.`)
     }
 

packages/feathers/src/service.ts

@@ -48,6 +48,10 @@ export function getHookMethods(service: any, options: ServiceOptions) {
     .concat(methods)
 }
 
+export function getExternalMethods(options: ServiceOptions): readonly string[] {
+  return options.externalMethods || options.methods || []
+}
+
 export function getServiceOptions(service: any): ServiceOptions {
   return service[SERVICE]
 }
@@ -57,12 +61,15 @@ export const normalizeServiceOptions = (service: any, options: ServiceOptions =
     methods = defaultServiceMethods.filter((method) => typeof service[method] === 'function'),
     events = service.events || []
   } = options
+  // externalMethods defaults to methods for backwards compatibility
+  const externalMethods = options.externalMethods || methods
   const serviceEvents = options.serviceEvents || defaultServiceEvents.concat(events)
 
   return {
     ...options,
     events,
     methods,
+    externalMethods,
     serviceEvents
   }
 }