addressed comments

Author: shylasummers

lib/msal-browser/apiReview/msal-browser.api.md

@@ -938,6 +938,8 @@ export interface IController {
     // (undocumented)
     initialize(request?: InitializeApplicationRequest): Promise<void>;
     // (undocumented)
+    initialize(request?: InitializeApplicationRequest, isBroker?: boolean): Promise<void>;
+    // (undocumented)
     initializeWrapperLibrary(sku: WrapperSKU, version: string): void;
     // @internal (undocumented)
     isBrowserEnv(): boolean;
@@ -993,7 +995,6 @@ export { INetworkModule }
 // @public
 export type InitializeApplicationRequest = {
     correlationId?: string;
-    isBroker?: boolean;
 };
 
 // Warning: (ae-missing-release-tag) "inMemRedirectUnavailable" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal)

lib/msal-browser/src/app/PublicClientApplication.ts

@@ -93,10 +93,7 @@ export class PublicClientApplication implements IPublicClientApplication {
      * @param request {?InitializeApplicationRequest}
      */
     async initialize(request?: InitializeApplicationRequest): Promise<void> {
-        return this.controller.initialize({
-            ...request,
-            isBroker: this.isBroker,
-        });
+        return this.controller.initialize(request, this.isBroker);
     }
 
     /**

lib/msal-browser/src/controllers/IController.ts

@@ -31,6 +31,11 @@ export interface IController {
     // TODO: Make request mandatory in the next major version?
     initialize(request?: InitializeApplicationRequest): Promise<void>;
 
+    initialize(
+        request?: InitializeApplicationRequest,
+        isBroker?: boolean
+    ): Promise<void>;
+
     acquireTokenPopup(request: PopupRequest): Promise<AuthenticationResult>;
 
     acquireTokenRedirect(request: RedirectRequest): Promise<void>;

lib/msal-browser/src/controllers/StandardController.ts

@@ -314,7 +314,10 @@ export class StandardController implements IController {
      * Initializer function to perform async startup tasks such as connecting to WAM extension
      * @param request {?InitializeApplicationRequest} correlation id
      */
-    async initialize(request?: InitializeApplicationRequest): Promise<void> {
+    async initialize(
+        request?: InitializeApplicationRequest,
+        isBroker?: boolean
+    ): Promise<void> {
         this.logger.trace("initialize called");
         if (this.initialized) {
             this.logger.info(
@@ -339,8 +342,11 @@ export class StandardController implements IController {
         );
         this.eventHandler.emitEvent(EventType.INITIALIZE_START);
 
-        if (!request?.isBroker) {
-            this.logMultipleInstances(initMeasurement);
+        // Broker applications are initialized twice, so we avoid double-counting it
+        if (!isBroker) {
+            try {
+                this.logMultipleInstances(initMeasurement);
+            } catch {}
         }
 
         await invokeAsync(
@@ -2397,6 +2403,7 @@ export class StandardController implements IController {
     ): void {
         const clientId = this.config.auth.clientId;
 
+        if (!window) return;
         // @ts-ignore
         window.msal = window.msal || {};
         // @ts-ignore
@@ -2405,26 +2412,14 @@ export class StandardController implements IController {
         // @ts-ignore
         const clientIds: string[] = window.msal.clientIds;
 
-        for (const currentId of clientIds) {
-            if (currentId === clientId) {
-                this.logger.warning(
-                    "There is already an instance of MSAL.js in the window with the same client id."
-                );
-                // @ts-ignore
-                window.msal.clientIds.push(clientId);
-                collectInstanceStats(clientId, performanceEvent);
-                return;
-            }
-        }
-
         if (clientIds.length > 0) {
-            this.logger.warning(
+            this.logger.verbose(
                 "There is already an instance of MSAL.js in the window."
             );
         }
         // @ts-ignore
         window.msal.clientIds.push(clientId);
-        collectInstanceStats(clientId, performanceEvent);
+        collectInstanceStats(clientId, performanceEvent, this.logger);
     }
 }
 

lib/msal-browser/src/request/InitializeApplicationRequest.ts

@@ -7,9 +7,7 @@
  * InitializeApplicationRequest: Request object passed by user to initialize application
  *
  * - correlationId              - Unique GUID set per request to trace a request end-to-end for telemetry purposes.
- * - isBroker                   - Boolean flag indicating whether the application is acting as a broker.
  */
 export type InitializeApplicationRequest = {
     correlationId?: string;
-    isBroker?: boolean;
 };

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

@@ -3,22 +3,27 @@
  * Licensed under the MIT License.
  */
 
-import { InProgressPerformanceEvent } from "@azure/msal-common/browser";
+import { InProgressPerformanceEvent, Logger } from "@azure/msal-common/browser";
 
 export function collectInstanceStats(
     currentClientId: string,
-    performanceEvent: InProgressPerformanceEvent
+    performanceEvent: InProgressPerformanceEvent,
+    logger: Logger
 ): void {
     const frameInstances: string[] =
         // @ts-ignore
         window.msal?.clientIds || [];
 
     const msalInstanceCount = frameInstances.length;
 
-    let sameClientIdInstanceCount = 0;
+    const sameClientIdInstanceCount = frameInstances.filter(
+        (i) => i === currentClientId
+    ).length;
 
-    for (const i of frameInstances) {
-        if (i === currentClientId) sameClientIdInstanceCount++;
+    if (sameClientIdInstanceCount > 1) {
+        logger.warning(
+            "There is already an instance of MSAL.js in the window with the same client id."
+        );
     }
     performanceEvent.add({
         msalInstanceCount: msalInstanceCount,

lib/msal-browser/test/app/PublicClientApplication.spec.ts

@@ -608,9 +608,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
                 "initialize"
             );
             await pca.initialize();
-            expect(initializeControllerSpy).toHaveBeenCalledWith({
-                isBroker: false,
-            });
+            expect(initializeControllerSpy).toHaveBeenCalledWith(undefined, false);
         });
     });
 
@@ -7794,7 +7792,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
             );
         });
 
-        it("Logs warning if there are two applications with different client ids in the same frame", async () => {
+        it("Logs verbose if there are two applications with different client ids in the same frame", async () => {
             const msalConfig: Configuration = {
                 auth: {
                     clientId: TEST_CONFIG.MSAL_CLIENT_ID,
@@ -7819,7 +7817,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
                 .mockImplementation();
             await pca2.initialize();
             expect(loggerCallbackStub).toHaveBeenCalledWith(
-                LogLevel.Warning,
+                LogLevel.Verbose,
                 expect.stringContaining(
                     "There is already an instance of MSAL.js in the window."
                 ),