From 164d75c0f11139a316baa91f1762de8f8eb7da2d Mon Sep 17 00:00:00 2001 From: David Luecke Date: Fri, 2 Dec 2022 09:07:45 -0800 Subject: [PATCH] fix(core): Improve service option usage and method option typings (#2902) --- packages/authentication-oauth/src/index.ts | 4 ++-- packages/authentication-oauth/src/utils.ts | 2 +- packages/feathers/src/application.ts | 4 ++-- packages/feathers/src/declarations.ts | 19 ++++++++++++++++--- packages/feathers/src/service.ts | 16 ++++++++-------- packages/feathers/test/application.test.ts | 4 ++-- packages/feathers/test/declarations.test.ts | 7 +++++-- packages/schema/test/fixture.ts | 9 ++------- .../transport-commons/src/channels/mixins.ts | 4 ++-- 9 files changed, 40 insertions(+), 29 deletions(-) diff --git a/packages/authentication-oauth/src/index.ts b/packages/authentication-oauth/src/index.ts index 9d2099e536..b4971674e5 100644 --- a/packages/authentication-oauth/src/index.ts +++ b/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) diff --git a/packages/authentication-oauth/src/utils.ts b/packages/authentication-oauth/src/utils.ts index cd6f5edfde..6f03e78b9c 100644 --- a/packages/authentication-oauth/src/utils.ts +++ b/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 => { diff --git a/packages/feathers/src/application.ts b/packages/feathers/src/application.ts index c0997d1f56..88ced8e0cd 100644 --- a/packages/feathers/src/application.ts +++ b/packages/feathers/src/application.ts @@ -145,7 +145,7 @@ export class Feathers use( path: L, service: keyof any extends keyof Services ? ServiceInterface | Application : Services[L], - options?: ServiceOptions + options?: ServiceOptions ): this { if (typeof path !== 'string') { throw new Error(`'${path}' is not a valid service path.`) @@ -163,7 +163,7 @@ export class Feathers 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) { diff --git a/packages/feathers/src/declarations.ts b/packages/feathers/src/declarations.ts index f7880e8f06..cf6ee927f0 100644 --- a/packages/feathers/src/declarations.ts +++ b/packages/feathers/src/declarations.ts @@ -19,10 +19,23 @@ export interface Paginated { /** * Options that can be passed when registering a service via `app.use(name, service, options)` */ -export interface ServiceOptions { +export interface ServiceOptions { + /** + * 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 { use( path: L, service: keyof any extends keyof Services ? ServiceInterface | Application : Services[L], - options?: ServiceOptions + options?: ServiceOptions ): this /** diff --git a/packages/feathers/src/service.ts b/packages/feathers/src/service.ts index 269cf2bf3b..febe0a81a4 100644 --- a/packages/feathers/src/service.ts +++ b/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}\``) diff --git a/packages/feathers/test/application.test.ts b/packages/feathers/test/application.test.ts index ec2e642564..5160972dfe 100644 --- a/packages/feathers/test/application.test.ts +++ b/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 }) diff --git a/packages/feathers/test/declarations.test.ts b/packages/feathers/test/declarations.test.ts index acca57b7f2..1ee2389dc7 100644 --- a/packages/feathers/test/declarations.test.ts +++ b/packages/feathers/test/declarations.test.ts @@ -70,7 +70,9 @@ describe('Feathers typings', () => { const app2 = feathers, 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')) diff --git a/packages/schema/test/fixture.ts b/packages/schema/test/fixture.ts index 6855bb7aeb..9a4ed01270 100644 --- a/packages/schema/test/fixture.ts +++ b/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 } })) diff --git a/packages/transport-commons/src/channels/mixins.ts b/packages/transport-commons/src/channels/mixins.ts index f60df6fe73..e0f52b15c8 100644 --- a/packages/transport-commons/src/channels/mixins.ts +++ b/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`)