cmd-johnson / deno-oauth2-client

Minimalistic OAuth 2.0 client for Deno.
MIT License
45 stars 9 forks source link

fix: nullish syntax error #15

Closed alexanderschau closed 3 years ago

alexanderschau commented 3 years ago

After bundeling this package with deno bundle, the compiler throws a syntax error because of a combination of a nullish coalescing operator (??) and a AND operator (&&), which is prohibited . This PR replaces the ?? with ||. Another alternative would be the following snippet:

let stateValidator: (((s: string | null) => boolean) | undefined | "") =
  options.stateValidator;
if (!stateValidator) {
  stateValidator = (options.state && ((s: string | null) =>
    s === options.state));
}
if (!stateValidator) {
  stateValidator = this.client.config.defaults?.stateValidator;
}

But I think the current version should work fine.

cmd-johnson commented 3 years ago

Thank you for the PR!

According to the article you linked, my usage of the nullish coalescing operator should be perfectly fine, as I'm wrapping the part with && in parentheses. However, Deno appears to drop these parentheses when running deno bundle. Looks like this issue was reported and apparently fixed in December 2020. However, with Deno 1.10.1, I'm able to reproduce this, so I reported this as a regression.

In the meantime, I'll merge this PR (right after I fixed the errors reported by the CI in the rest of the code that are due to Deno changes, I assume). In this case it really doesn't make any difference whether || or ?? is used.

cmd-johnson commented 3 years ago

Merged in f4c7f2d678f333009c7ebeec5ccb9a2c22a975c8 and released in v0.2.1 (https://deno.land/x/oauth2_client@v0.2.1).

cmd-johnson commented 3 years ago

Looks like I messed up a bit when rebasing and merging this manually, so I'll just close this PR. Rest assured that your contribution is still attributed to you (cd49190658a44c07df1e3eb937e87ae86fd93fbd).