Skip to content

chore: Updated typings - #2384

Merged
daffl merged 1 commit into
feathersjs:dovefrom
deskoh:pr
Jun 8, 2021
Merged

daffl merged 1 commit into
feathersjs:dovefrom
deskoh:pr

Conversation

@deskoh

@deskoh deskoh commented Jun 8, 2021

Copy link
Copy Markdown
Contributor

Summary

The current generated type definitions for JwtStrategy is as follows:

// jwt.d.ts
parse(req: IncomingMessage): Promise<{
  strategy: string;
  accessToken: string;
}>;

This backwards-compatible PR updates the type definition to:

 parse(req: IncomingMessage): Promise<{
    strategy: string;
    accessToken: string;
} | null>;

This is to allow other projects with strictNullChecks to sub-class JwtStrategy and able to compile without any errors.

node_modules/feathers-authentication-oidc/lib/OidcStrategy.d.ts:9:5 - error TS2416: Property 'parse' in type 'OidcStrategy' is not assignable to the same property in base type 'JWTStrategy'.
  Type '(req: IncomingMessage) => Promise<{ strategy: string; accessToken: string; } | null>' is not assignable to type '(req: IncomingMessage) => Promise<{ strategy: string; accessToken: string; }>'.
    Type 'Promise<{ strategy: string; accessToken: string; } | null>' is not assignable to type 'Promise<{ strategy: string; accessToken: string; }>'.
      Type '{ strategy: string; accessToken: string; } | null' is not assignable to type '{ strategy: string; accessToken: string; }'.
        Type 'null' is not assignable to type '{ strategy: string; accessToken: string; }'.

9     parse(req: IncomingMessage): Promise<{

@daffl

daffl commented Jun 8, 2021

Copy link
Copy Markdown
Member

Great, thank you! Hopefully we can eventually get everything (including the internal codebase) working with strictNullChecks, too. Will go out in the next prerelease.

@daffl
daffl merged commit b6a2672 into feathersjs:dove Jun 8, 2021
@deskoh
deskoh deleted the pr branch June 8, 2021 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants