Skip to content

Commit

Permalink
feat(typescript): Improve params and query typeability (#2600)
Browse files Browse the repository at this point in the history
  • Loading branch information
daffl authored Apr 20, 2022
1 parent 97313e1 commit df28b76
Show file tree
Hide file tree
Showing 24 changed files with 234 additions and 123 deletions.
11 changes: 7 additions & 4 deletions packages/adapter-commons/src/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,16 @@ const alwaysMulti: { [key: string]: boolean } = {
update: false
};

export interface PaginationOptions {
default?: number;
max?: number;
}

export interface ServiceOptions {
events?: string[];
multi?: boolean|string[];
id?: string;
paginate?: {
default?: number;
max?: number;
}
paginate?: PaginationOptions
/**
* @deprecated renamed to `allow`.
*/
Expand All @@ -38,6 +40,7 @@ export interface AdapterOptions<M = any> extends Pick<ServiceOptions, 'multi'|'a

export interface AdapterParams<M = any> extends Params {
adapter?: Partial<AdapterOptions<M>>;
paginate?: false|PaginationOptions;
}

/**
Expand Down
6 changes: 3 additions & 3 deletions packages/authentication-oauth/src/strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// @ts-ignore
import querystring from 'querystring';
import {
AuthenticationRequest, AuthenticationBaseStrategy, AuthenticationResult
AuthenticationRequest, AuthenticationBaseStrategy, AuthenticationResult, AuthenticationParams
} from '@feathersjs/authentication';
import { Params } from '@feathersjs/feathers';
import { NotAuthenticated } from '@feathersjs/errors';
Expand Down Expand Up @@ -84,7 +84,7 @@ export class OAuthStrategy extends AuthenticationBaseStrategy {
return redirect;
}

async getRedirect (data: AuthenticationResult|Error, params?: Params): Promise<string | null> {
async getRedirect (data: AuthenticationResult|Error, params?: AuthenticationParams): Promise<string | null> {
const queryRedirect = (params && params.redirect) || '';
const redirect = await this.getAllowedOrigin(params);

Expand Down Expand Up @@ -156,7 +156,7 @@ export class OAuthStrategy extends AuthenticationBaseStrategy {
});
}

async authenticate (authentication: AuthenticationRequest, originalParams: Params) {
async authenticate (authentication: AuthenticationRequest, originalParams: AuthenticationParams) {
const entity: string = this.configuration.entity;
const { provider, ...params } = originalParams;
const profile = await this.getProfile(authentication, params);
Expand Down
6 changes: 3 additions & 3 deletions packages/authentication-oauth/test/fixture.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { feathers, Params } from '@feathersjs/feathers';
import { feathers } from '@feathersjs/feathers';
import express, { rest, errorHandler } from '@feathersjs/express';
import { memory } from '@feathersjs/memory';
import { AuthenticationService, JWTStrategy, AuthenticationRequest } from '@feathersjs/authentication';
import { AuthenticationService, JWTStrategy, AuthenticationRequest, AuthenticationParams } from '@feathersjs/authentication';
import { express as oauth, OAuthStrategy } from '../src';

export class TestOAuthStrategy extends OAuthStrategy {
async authenticate (data: AuthenticationRequest, params: Params) {
async authenticate (data: AuthenticationRequest, params: AuthenticationParams) {
const { fromMiddleware } = params;
const authResult = await super.authenticate(data, params);

Expand Down
12 changes: 10 additions & 2 deletions packages/authentication/src/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ export interface AuthenticationRequest {
[key: string]: any;
}

export interface AuthenticationParams extends Params {
payload?: { [key: string]: any };
jwtOptions?: SignOptions;
authStrategies?: string[];
secret?: string;
[key: string]: any;
}

export type ConnectionEvent = 'login' | 'logout' | 'disconnect';

export interface AuthenticationStrategy {
Expand Down Expand Up @@ -57,7 +65,7 @@ export interface AuthenticationStrategy {
* @param authentication The authentication request
* @param params The service call parameters
*/
authenticate? (authentication: AuthenticationRequest, params: Params): Promise<AuthenticationResult>;
authenticate? (authentication: AuthenticationRequest, params: AuthenticationParams): Promise<AuthenticationResult>;
/**
* Update a real-time connection according to this strategy.
*
Expand Down Expand Up @@ -232,7 +240,7 @@ export class AuthenticationBase {
* @param params Service call parameters
* @param allowed A list of allowed strategy names
*/
async authenticate (authentication: AuthenticationRequest, params: Params, ...allowed: string[]) {
async authenticate (authentication: AuthenticationRequest, params: AuthenticationParams, ...allowed: string[]) {
const { strategy } = authentication || {};
const [ authStrategy ] = this.getStrategies(strategy);
const strategyAllowed = allowed.includes(strategy);
Expand Down
1 change: 1 addition & 0 deletions packages/authentication/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export {
AuthenticationRequest,
AuthenticationResult,
AuthenticationStrategy,
AuthenticationParams,
ConnectionEvent
} from './core';
export { AuthenticationBaseStrategy } from './strategy';
Expand Down
4 changes: 2 additions & 2 deletions packages/authentication/src/jwt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { createDebug } from '@feathersjs/commons';
import lt from 'long-timeout';

import { AuthenticationBaseStrategy } from './strategy';
import { AuthenticationRequest, AuthenticationResult, ConnectionEvent } from './core';
import { AuthenticationParams, AuthenticationRequest, AuthenticationResult, ConnectionEvent } from './core';

const debug = createDebug('@feathersjs/authentication/jwt');
const SPLIT_HEADER = /(\S+)\s+(\S+)/;
Expand Down Expand Up @@ -116,7 +116,7 @@ export class JWTStrategy extends AuthenticationBaseStrategy {
return authResult.authentication.payload.sub;
}

async authenticate (authentication: AuthenticationRequest, params: Params) {
async authenticate (authentication: AuthenticationRequest, params: AuthenticationParams) {
const { accessToken } = authentication;
const { entity } = this.configuration;

Expand Down
14 changes: 7 additions & 7 deletions packages/authentication/src/service.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import merge from 'lodash/merge';
import { NotAuthenticated } from '@feathersjs/errors';
import { AuthenticationBase, AuthenticationResult, AuthenticationRequest } from './core';
import { AuthenticationBase, AuthenticationResult, AuthenticationRequest, AuthenticationParams } from './core';
import { connection, event } from './hooks';
import '@feathersjs/transport-commons';
import { createDebug } from '@feathersjs/commons';
import { Params, ServiceMethods, ServiceAddons } from '@feathersjs/feathers';
import { ServiceMethods, ServiceAddons } from '@feathersjs/feathers';
import jsonwebtoken from 'jsonwebtoken';

const debug = createDebug('@feathersjs/authentication/service');
Expand All @@ -29,7 +29,7 @@ declare module '@feathersjs/feathers/lib/declarations' {
// eslint-disable-next-line
export interface AuthenticationService extends ServiceAddons<AuthenticationResult, AuthenticationResult> {}

export class AuthenticationService extends AuthenticationBase implements Partial<ServiceMethods<AuthenticationResult>> {
export class AuthenticationService extends AuthenticationBase implements Partial<ServiceMethods<AuthenticationResult, AuthenticationRequest, AuthenticationParams>> {
constructor (app: any, configKey = 'authentication', options = {}) {
super(app, configKey, options);

Expand All @@ -51,7 +51,7 @@ export class AuthenticationService extends AuthenticationBase implements Partial
* @param _authResult The current authentication result
* @param params The service call parameters
*/
async getPayload (_authResult: AuthenticationResult, params: Params) {
async getPayload (_authResult: AuthenticationResult, params: AuthenticationParams) {
// Uses `params.payload` or returns an empty payload
const { payload = {} } = params;

Expand All @@ -65,7 +65,7 @@ export class AuthenticationService extends AuthenticationBase implements Partial
* @param authResult The authentication result
* @param params Service call parameters
*/
async getTokenOptions (authResult: AuthenticationResult, params: Params) {
async getTokenOptions (authResult: AuthenticationResult, params: AuthenticationParams) {
const { service, entity, entityId } = this.configuration;
const jwtOptions = merge({}, params.jwtOptions, params.jwt);
const value = service && entity && authResult[entity];
Expand All @@ -92,7 +92,7 @@ export class AuthenticationService extends AuthenticationBase implements Partial
* @param data The authentication request (should include `strategy` key)
* @param params Service call parameters
*/
async create (data: AuthenticationRequest, params?: Params) {
async create (data: AuthenticationRequest, params?: AuthenticationParams) {
const authStrategies = params.authStrategies || this.configuration.authStrategies;

if (!authStrategies.length) {
Expand Down Expand Up @@ -131,7 +131,7 @@ export class AuthenticationService extends AuthenticationBase implements Partial
* @param id The JWT to remove or null
* @param params Service call parameters
*/
async remove (id: string | null, params?: Params) {
async remove (id: string | null, params?: AuthenticationParams) {
const { authentication } = params;
const { authStrategies } = this.configuration;

Expand Down
14 changes: 7 additions & 7 deletions packages/authentication/test/hooks/event.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import assert from 'assert';
import { feathers, Params, HookContext } from '@feathersjs/feathers';
import { feathers, HookContext } from '@feathersjs/feathers';

import hook from '../../src/hooks/event';
import { AuthenticationRequest, AuthenticationResult } from '../../src/core';
import { AuthenticationParams, AuthenticationRequest, AuthenticationResult } from '../../src/core';

describe('authentication/hooks/events', () => {
const app = feathers().use('/authentication', {
const app = feathers().use('authentication', {
async create (data: AuthenticationRequest) {
return data;
},
Expand All @@ -27,7 +27,7 @@ describe('authentication/hooks/events', () => {
message: 'test'
};

app.once('login', (result: AuthenticationResult, params: Params, context: HookContext) => {
app.once('login', (result: AuthenticationResult, params: AuthenticationParams, context: HookContext) => {
try {
assert.deepStrictEqual(result, data);
assert.ok(params.testParam);
Expand All @@ -41,11 +41,11 @@ describe('authentication/hooks/events', () => {
service.create(data, {
testParam: true,
provider: 'test'
});
}as any);
});

it('logout', done => {
app.once('logout', (result: AuthenticationResult, params: Params, context: HookContext) => {
app.once('logout', (result: AuthenticationResult, params: AuthenticationParams, context: HookContext) => {
try {
assert.deepStrictEqual(result, {
id: 'test'
Expand All @@ -61,7 +61,7 @@ describe('authentication/hooks/events', () => {
service.remove('test', {
testParam: true,
provider: 'test'
});
} as any);
});

it('does nothing when provider is not set', done => {
Expand Down
4 changes: 2 additions & 2 deletions packages/express/src/declarations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ interface ExpressUseHandler<T, Services> {
<L extends keyof Services & string> (
path: L,
...middlewareOrService: (
Express|express.RequestHandler|
Express|express.RequestHandler|express.RequestHandler[]|
(keyof any extends keyof Services ? ServiceInterface : Services[L])
)[]
): T;
Expand Down Expand Up @@ -44,7 +44,7 @@ declare module '@feathersjs/feathers/lib/declarations' {

declare module 'express-serve-static-core' {
interface Request {
feathers?: Partial<FeathersParams>;
feathers?: Partial<FeathersParams> & { [key: string]: any };
lookup?: RouteLookup;
}

Expand Down
7 changes: 3 additions & 4 deletions packages/express/test/rest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,10 +380,9 @@ describe('@feathersjs/express/rest provider', () => {
it('allows middleware arrays before and after a service', async () => {
const app = expressify(feathers());

app
.use(express.json())
.configure(rest())
.use('/todo', [function (req: Request, _res: Response, next: NextFunction) {
app.use(express.json());
app.configure(rest());
app.use('/todo', [function (req: Request, _res: Response, next: NextFunction) {
req.body.before = ['before first'];
next();
}, function (req: Request, _res: Response, next: NextFunction) {
Expand Down
Loading

0 comments on commit df28b76

Please # to comment.