Update README.md + code quality comments

[haf]

It's really hard to follow your code examples; I tried to clarify some of the flows in this PR. I also have some comments on the code-base in general:

Also, it's against "best practises" to use undefined as a value, because it's possible to write to that symbol globally https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/undefined

Also, generally, the code in this repo is hard to follow because the samples are with very mutable state; it would be much better to have clear input-output functions, so that the flow and mutation of variables can be followed.

You also have some functions that via runtime-reasoning cannot produce undefined behaviour, but that are not type-safe (e.g. makeTokenRequest in the case that code == null && request == null to name the most obvious example). There's also a bit of copying going on from objects to other objects; you should decide whether you should be OOP (in which case you either use the requests/responses as immutable value objects that you perform operations with, and you put the logic of the copies in instance methods on those value objects; OR you should do it with mutable objects; so instead of reading select properties from one token response to another (request) object, create an instance method on the response object, taking the request object, filling it up. That way you encapsulate its behaviour. This is similar to how you'd do with value objects (better!), but then the method would return a new object with the new values instead.

The way this code is built is by mutating instance variables in the App class (as seen from a consumer perspective); it would be better if you make the flow explicit both with the values/variables (not fields), state (mutually recursive fn or a switch-statement) and side-effects (via convention; right now the verb prefixes are a bit all over the map, considering you're also feeding the public class types with "requestors" — does then the public class type do the side-effect, or is that delegated to the requestor? [rethorical question])

Another example:

response => {
          let isFirstRequest = false;
          if (this.tokenResponse) {
            // copy over new fields
            this.tokenResponse.accessToken = response.accessToken;
            this.tokenResponse.issuedAt = response.issuedAt;
            this.tokenResponse.expiresIn = response.expiresIn;
            this.tokenResponse.tokenType = response.tokenType;
            this.tokenResponse.scope = response.scope;
          } else {
            isFirstRequest = true;
            this.tokenResponse = response;
          }
}

In this case it's unclear to the reader why we can't just overwrite the tokenResponse with the new response value; as this would have the correct effect of updating the token fields (accessToken, issuedAt, expiresIn, ...) in the first case, and in the second case it would be identical to the current implementation.

Or in this case:

 } else if (this.tokenResponse) {
  request = // new refresh token request

however; since the refresh token may be missing (it's optional after all), this code-path will crash unless the STS/AS/OIDC server always sends a refresh token.

I also think it would be a good idea to make a note about offline scope (== ability to get a refresh token), as well as how to write the loop that keeps the access token fresh — this is something oidc-client does very well, which helps its uptake:

  // snip...

  // --------- 2. Let AppAuthJS read state, if needed
  if (isCallbackURI) {
    console.log("Calling completeAuthorizationRequestIfPossible()")
    await authReqHandler.completeAuthorizationRequestIfPossible()
  }

  let prevTokenRes: TokenResponse | null =
    tokenResponseJSON == null
      ? null
      : new TokenResponse(tokenResponseJSON)

  console.log("Starting loop...")
  while (!ct.isCancelleled) {
    if (code == null && prevTokenRes == null) {
      console.log("Calling fetchAuthorization(config, ", authzReq, ")")
      authReqHandler.performAuthorizationRequest(config, authzReq)
      return;
    }

    // --------- 3. Keep the accessToken alive
    if (shouldMakeTokenCall(code, prevTokenRes)) {
      // @ts-ignore The error is wrong; the above while loop will await either code xor prevTokenRes
      const tokenReq = nextTokenReq(redirectURI, code != null ? code : prevTokenRes, authzReq)
      console.log(`Calling performTokenRequest(grantType=${tokenReq.grantType})`)
      prevTokenRes = await tokenHandler.performTokenRequest(config, tokenReq)
      code = null
      setTokenResponseJSON(prevTokenRes.toJson())
    }

    await sleep(2000)
  }

[tikurahul]

Please write a new sample in a separate repo and I can link to it.

[haf]

@tikurahul I think you've misunderstanding. This is not a new sample, this PR is about improvements to your README. Perhaps you didn't read the text?

[tikurahul]

I did read them. I don't think they make a difference to the readability.