Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(typescript): Make b64 header optional #324

Merged
merged 1 commit into from
Nov 12, 2021
Merged

Conversation

bespokebob
Copy link
Contributor

The b64 header is currently defined as a required field, so omitting it throws a typescript error. Since its type is defined as "never", there is nothing that can be assigned to it, making it impossible to actually use.
This makes b64 optional, which still prevents assigning to it due to the type.

The b64 header is currently defined as a required field, so omitting it throws a typescript error. Since its type is defined as "never", there is nothing that can be assigned to it, making it impossible to actually use.
This makes b64 optional, which still prevents assigning to it due to the type.
@bespokebob
Copy link
Contributor Author

bespokebob commented Nov 12, 2021

Error before fix, when b64 is omitted:

Argument of type '{ alg: string; }' is not assignable to parameter of type 'JWTHeaderParameters'.
  Property 'b64' is missing in type '{ alg: string; }' but required in type 'JWTHeaderParameters'.ts(2345)

(Correct) error trying to assign to b64 before fix:

Type 'boolean' is not assignable to type 'never'.ts(2322)
types.d.ts(265, 3): The expected type comes from property 'b64' which is declared here on type 'JWTHeaderParameters'

Attempting to assign to b64 after fix:

Type 'false' is not assignable to type 'undefined'.ts(2322)
types.d.ts(265, 3): The expected type comes from property 'b64' which is declared here on type 'JWTHeaderParameters'

Which is an odd error, but I guess TypeScript is treating b64?: never as never | undefined which would be the same as undefined? Actually defining that field as b64: undefined still gives the "required" error, though.

@panva
Copy link
Owner

panva commented Nov 12, 2021

@youngbob thank you for catching this, i'll release a fix with this as soon as CI passes.

@panva panva added the full-ci label Nov 12, 2021
@github-actions github-actions bot removed the full-ci label Nov 12, 2021
@panva panva merged commit 9da0a7f into panva:main Nov 12, 2021
@panva
Copy link
Owner

panva commented Nov 12, 2021

@CMCDragonkai
Copy link

I had some code inherited that set the b64 to true. Now TSC is complaining that true is not assignable type undefined. How is this supposed to work?

@panva
Copy link
Owner

panva commented Nov 18, 2021

Ah yeah. Only false should be forbidden for JWTs... will fix. Nevertheless setting it to true is the same as omitting it.

@panva
Copy link
Owner

panva commented Nov 18, 2021

@github-actions github-actions bot locked and limited conversation to collaborators Feb 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants