Bump typescript to `4.9.5` and build target to `es2020` #5750

konstantin-msft · 2023-03-02T18:18:41Z

Bump typescript to 4.9.5.
Bump target/module versions to es2020 when applicable.
Replace calls to deprecated mocked() from ts-jest/utils with jest.spyOn().
Fix types' coercions to facilitate compatibility with the new ts version.
Make generic type extend ServerAuthorizationTokenResponse for NetworkManager.sendPostRequest() in msal-common to handle post processing properly.
Make serverTelemetryManager.cacheFailedRequest() accept any types of errors.

konstantin-msft · 2023-03-02T18:54:39Z

lib/msal-common/src/account/AuthToken.ts

@@ -44,7 +44,7 @@ export class AuthToken {
            const base64Decoded = crypto.base64Decode(base64TokenPayload);
            return JSON.parse(base64Decoded) as TokenClaims;
        } catch (err) {
-            throw ClientAuthError.createTokenParsingError(err);
+            throw ClientAuthError.createTokenParsingError(err as string);


Type coercions are prohibited in ts v4.

konstantin-msft · 2023-03-02T18:55:28Z

lib/msal-node/src/client/ClientApplication.ts

@@ -162,8 +162,8 @@ export abstract class ClientApplication {
        } catch (e) {
            if (e instanceof AuthError) {
                e.setCorrelationId(validRequest.correlationId);
+                serverTelemetryManager.cacheFailedRequest(e);


serverTelemetryManager.cacheFailedRequest(e) expects an instance of AuthError. We can't pass an arbitrary error.

If serverTelemetryManager.cacheFailedRequest() is able to accept any types of error, do these need to be reverted? (and below)

I have already reverted it but forgot to resolve the comment. Sorry about that.

But cacheFailedRequest is still being called inside the if (e instanceof AuthError) block

Completely forgot about msal-node module. Good catch, thank you. Fixed.

konstantin-msft · 2023-03-02T18:56:52Z

lib/msal-node/test/client/ClientAssertion.spec.ts

@@ -1,11 +1,11 @@
 import { ClientAssertion } from "../../src/client/ClientAssertion";
 import { TEST_CONSTANTS } from "../utils/TestConstants";
 import { CryptoProvider } from "../../src/crypto/CryptoProvider";
-import { sign } from "jsonwebtoken";
-import { mocked } from "ts-jest/utils";


konstantin-msft · 2023-03-02T18:58:20Z

lib/msal-common/src/network/NetworkManager.ts

@@ -31,7 +32,7 @@ export class NetworkManager {
     * @param tokenEndpoint
     * @param options
     */
-    async sendPostRequest<T>(thumbprint: RequestThumbprint, tokenEndpoint: string, options: NetworkRequestOptions): Promise<NetworkResponse<T>> {
+    async sendPostRequest<T extends ServerAuthorizationTokenResponse>(thumbprint: RequestThumbprint, tokenEndpoint: string, options: NetworkRequestOptions): Promise<NetworkResponse<T>> {


T must extend ServerAuthorizationTokenResponse because ThrottlingUtils.postProcess() expects it as an input.

codecov-commenter · 2023-03-02T19:44:11Z

Codecov Report

❗ No coverage uploaded for pull request base (msal-browser-v3@8b576a2). Click here to learn what that means.
The diff coverage is n/a.

Flag	Coverage Δ	*Carryforward flag
msal-common	`85.32% <0.00%> (?)`
msal-node-extensions	`45.81% <0.00%> (?)`	Carriedforward from 185b755
msal-react	`94.82% <0.00%> (?)`
node-token-validation	`87.28% <0.00%> (?)`

*This pull request uses carry forward flags. Click here to find out more.

konstantin-msft · 2023-03-06T16:08:34Z

lib/msal-react/tsconfig.json

@@ -2,6 +2,7 @@
  "include": ["src", "types", "test"],
  "compilerOptions": {
    "module": "esnext",
+    "target": "esnext",


Required for proper class transpiling.

peterzenz

I have the questions I mentioned in teams about the explicit type changes for TS4 and how those resolve. I'd also like some clarity on the ts-ignores as to what it would take to address those correctly. If we want this in though now to unblock I'm OK, but please track bugs/work items around these things if they are unresolved.

peterzenz · 2023-03-09T19:45:18Z

lib/msal-browser/src/interaction_client/PopupClient.ts

@@ -190,9 +191,9 @@ export class PopupClient extends StandardInteractionClient {

            if (e instanceof AuthError) {
                (e as AuthError).setCorrelationId(this.correlationId);
+                serverTelemetryManager.cacheFailedRequest(e);


This looks like a functional change and not a change related to the TS/ES intent of this PR. You mention type coercion changes are not supported in the new TS and I'm seeing this pattern throughout this change. What happened in the original code if we called .cacheFailedRequest(e) and it was not of type AuthError? Did the failure cascade out from cacheFailedRequest or was it able to coerce whatever error type that was thrown into something usable? If it's not of type AuthError we might still want to cache a failed response (failure is failure...). I just don't know what other types we might get in this flow.

Same comment, similar pattern below.

That's a very good point. I have made serverTelemetryManager.cacheFailedRequest() to accept any types of errors to make sure we cache all of them.

peterzenz · 2023-03-09T20:46:48Z

lib/msal-browser/src/utils/BrowserUtils.ts

     */
    static getBrowserNetworkClient(): INetworkModule {
+        // @ts-ignore TS2774


Maybe a comment on why we're adding the ts-ignore here? Is there a future fix we're holding off on just to unblock folks?

As I look at the other ts-ignores around window I'm thrown back to some earlier discussions on web workers not supporting window references. @sameerag or @tnorling I thought there were some alternates here that might be more friendly. I'll admit though I'm not up to speed on the new TS/ES changes that might impact here.

Reading this, I get that we are checking if window.fetch and window.headers are defined, what is the alternative here?

May be a try .. catch where if window.fetch isn't defined, we cannot call the fetchClient anyway. I am not sure if that actually throws though but worth a try.

Unfortunately, try .. catch won't help here.

peterzenz · 2023-03-09T20:53:38Z

lib/msal-browser/test/interaction_client/RedirectClient.spec.ts

@@ -6,7 +6,33 @@
 import sinon from "sinon";
 import { PublicClientApplication } from "../../src/app/PublicClientApplication";
 import { TEST_CONFIG, TEST_URIS, TEST_HASHES, TEST_TOKENS, TEST_DATA_CLIENT_INFO, TEST_TOKEN_LIFETIMES, RANDOM_TEST_GUID, DEFAULT_OPENID_CONFIG_RESPONSE, testNavUrl, TEST_STATE_VALUES, DEFAULT_TENANT_DISCOVERY_RESPONSE, testLogoutUrl, TEST_SSH_VALUES } from "../utils/StringConstants";
-import { ServerError, Constants, AccountInfo, TokenClaims, AuthenticationResult, CommonAuthorizationCodeRequest, CommonAuthorizationUrlRequest, AuthToken, PersistentCacheKeys, AuthorizationCodeClient, ResponseMode, ProtocolUtils, AuthenticationScheme, Logger, ServerTelemetryEntity, LogLevel, NetworkResponse, ServerAuthorizationTokenResponse, CcsCredential, CcsCredentialType, CommonEndSessionRequest, ServerTelemetryManager, AccountEntity, ClientConfigurationError } from "@azure/msal-common";
+import {


Not as terse, but so much more readable. :)

At some point I had all files like this. Can we make this a standard?

… of errors.

jo-arroyo · 2023-03-10T18:45:58Z

change/msal-1a69cf50-db5e-4aaf-ab86-32f98e28bec4.json

+{
+  "type": "minor",
+  "comment": "Bump typescript to 4.9.5 and target/module to es2020 #5750",
+  "packageName": "msal",


If changes reverted, shouldn't this file be deleted?

extensions/msal-node-extensions/tsconfig.json

…`msal-node`.

jo-arroyo

Thanks for addressing comments!

- Bump `typescript` to 4.9.5. - Bump target/module versions to `es2020` when applicable. - Replace calls to deprecated `mocked()` from `ts-jest/utils` with `jest.spyOn()`. - Fix types' coercions to facilitate compatibility with the new ts version. - Make generic type extend `ServerAuthorizationTokenResponse` for `NetworkManager.sendPostRequest()` in `msal-common` to handle post processing properly. - Make `serverTelemetryManager.cacheFailedRequest()` accept any types of errors.

ghost · 2023-06-19T21:25:07Z

🎉@azure/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

konstantin-msft changed the title ~~Bump typescript to 4.9.5 and ecma build target to es2020.~~ Bump typescript to 4.9.5 and ecma build target to es2020 Mar 2, 2023

konstantin-msft changed the base branch from dev to msal-browser-v3 March 2, 2023 18:52

konstantin-msft force-pushed the 2069323_bump_ts_to_v4 branch from 5b23bd2 to c6c6ecb Compare March 2, 2023 18:53

github-actions bot removed the samples Related to the samples apps for the library. label Mar 2, 2023

konstantin-msft commented Mar 2, 2023

View reviewed changes

github-actions bot removed the msal-react Related to @azure/msal-react label Mar 2, 2023

konstantin-msft force-pushed the 2069323_bump_ts_to_v4 branch from 99a26cf to eba9ef4 Compare March 3, 2023 02:12

github-actions bot added the msal-react Related to @azure/msal-react label Mar 3, 2023

konstantin-msft commented Mar 6, 2023

View reviewed changes

konstantin-msft force-pushed the 2069323_bump_ts_to_v4 branch from 415441b to 288d07a Compare March 7, 2023 20:16

konstantin-msft marked this pull request as ready for review March 7, 2023 20:54

konstantin-msft requested review from jennyf19, jmprieur, bgavrilMS, tnorling, jo-arroyo, peterzenz, sameerag and hectormmg as code owners March 7, 2023 20:54

konstantin-msft added 15 commits March 9, 2023 13:22

- Fix type coercions in msal-browser.

e761062

- Revert msal-react changes.

b41723a

- Fix msal-browser tests.

ec75ff4

- Bump typescript version in msal-react.

0853c34

- Set msal-react compiler target to esnext.

3fc48d2

- Fix type coercion in msal-react tests.

668d3ac

Change files

eeaeb7d

Change files

c97e860

- Bump typescript to 4.9.5 for msal-node-extensions.

b1cfee9

Change files

691c819

- Update change files.

700fa36

- Set msal-react build target to es2020.

8402de3

- Revert msal-core changes.

5347c92

Revert msal-core changes

0ed32fa

- Remove redundant instanceof condition.

629d84d

konstantin-msft force-pushed the 2069323_bump_ts_to_v4 branch from a71ef64 to 629d84d Compare March 9, 2023 18:23

konstantin-msft requested a review from tnorling March 9, 2023 18:41

peterzenz approved these changes Mar 9, 2023

View reviewed changes

- Make serverTelemetryManager.cacheFailedRequest() accept any types…

185b755

… of errors.

peterzenz approved these changes Mar 10, 2023

View reviewed changes

jo-arroyo reviewed Mar 10, 2023

View reviewed changes

konstantin-msft added 2 commits March 10, 2023 14:13

- Set build target to es2020 for msal-node-extensions.

9ab7b87

- Reverted serverTelemetryManager.cacheFailedRequest() changes for …

8c2f579

…`msal-node`.

konstantin-msft requested a review from jo-arroyo March 10, 2023 20:09

jo-arroyo approved these changes Mar 10, 2023

View reviewed changes

konstantin-msft merged commit 1e3e660 into msal-browser-v3 Mar 10, 2023

konstantin-msft deleted the 2069323_bump_ts_to_v4 branch March 10, 2023 23:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump typescript to `4.9.5` and build target to `es2020` #5750

Bump typescript to `4.9.5` and build target to `es2020` #5750

konstantin-msft commented Mar 2, 2023 •

edited

Loading

konstantin-msft Mar 2, 2023

konstantin-msft Mar 2, 2023 •

edited

Loading

jo-arroyo Mar 10, 2023

konstantin-msft Mar 10, 2023

jo-arroyo Mar 10, 2023 •

edited

Loading

konstantin-msft Mar 10, 2023

konstantin-msft Mar 2, 2023

konstantin-msft Mar 2, 2023

codecov-commenter commented Mar 2, 2023 •

edited

Loading

konstantin-msft Mar 6, 2023

peterzenz left a comment

peterzenz Mar 9, 2023

sameerag Mar 10, 2023

konstantin-msft Mar 10, 2023

peterzenz Mar 9, 2023

peterzenz Mar 9, 2023

sameerag Mar 10, 2023

konstantin-msft Mar 10, 2023

peterzenz Mar 9, 2023

sameerag Mar 10, 2023

jo-arroyo Mar 10, 2023

jo-arroyo left a comment

ghost commented Jun 19, 2023

Bump typescript to 4.9.5 and build target to es2020 #5750

Bump typescript to 4.9.5 and build target to es2020 #5750

Conversation

konstantin-msft commented Mar 2, 2023 • edited Loading

Choose a reason for hiding this comment

konstantin-msft Mar 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jo-arroyo Mar 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Mar 2, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

peterzenz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jo-arroyo left a comment

Choose a reason for hiding this comment

ghost commented Jun 19, 2023

Bump typescript to `4.9.5` and build target to `es2020` #5750

Bump typescript to `4.9.5` and build target to `es2020` #5750

konstantin-msft commented Mar 2, 2023 •

edited

Loading

konstantin-msft Mar 2, 2023 •

edited

Loading

jo-arroyo Mar 10, 2023 •

edited

Loading

codecov-commenter commented Mar 2, 2023 •

edited

Loading