@azure/msal-browser transitively includes @types/node via @azure/msal-common and breaks typings

[rjgotten]

trafficstars

Core Library

MSAL.js (@azure/msal-browser)

Core Library Version

2.33.0

Wrapper Library

Not Applicable

Wrapper Library Version

None

Public or Confidential Client?

Public

Description

When importing PublicClientApplication from @azure/msal-browser this will lead to an import chain that will go through @azure/msal-browser/dist/index.d.ts to several dependencies from @azure/msal-common - which will be imported through @azure/msal-common/dist/index.d.ts

The latter will export INativeBrokerPlugin:

which, contains a triple-slash import directive of the @types/node package:

This was verified via tsc --traceResolution :

Loading module as file / folder, candidate module location '<redacted>/node_modules/@azure/msal-common/dist/authority/Authority', target file type 'TypeScript'.
File '<redacted>/node_modules/@azure/msal-common/dist/authority/Authority.ts' does not exist.
File '<redacted>/node_modules/@azure/msal-common/dist/authority/Authority.tsx' does not exist.
File '<redacted>/node_modules/@azure/msal-common/dist/authority/Authority.d.ts' exist - use it as a name resolution result.
File '<redacted>/node_modules/@azure/msal-common/package.json' exists according to earlier cached lookups.
======== Module name './Authority' was successfully resolved to '<redacted>/node_modules/@azure/msal-common/dist/authority/Authority.d.ts' with Package ID '@azure/msal-common/dist/authority/[email protected]'. ========
======== Resolving module '../network/INetworkModule' from '<redacted>/node_modules/@azure/msal-common/dist/authority/AuthorityFactory.d.ts'. ========
Resolution for module '../network/INetworkModule' was found in cache from location '<redacted>/node_modules/@azure/msal-common/dist/authority'.
======== Module name '../network/INetworkModule' was successfully resolved to '<redacted>/node_modules/@azure/msal-common/dist/network/INetworkModule.d.ts' with Package ID '@azure/msal-common/dist/network/[email protected]'. ========
======== Resolving module '../cache/interface/ICacheManager' from '<redacted>/node_modules/@azure/msal-common/dist/authority/AuthorityFactory.d.ts'. ========
Resolution for module '../cache/interface/ICacheManager' was found in cache from location '<redacted>/node_modules/@azure/msal-common/dist/authority'.
======== Module name '../cache/interface/ICacheManager' was successfully resolved to '<redacted>/node_modules/@azure/msal-common/dist/cache/interface/ICacheManager.d.ts' with Package ID '@azure/msal-common/dist/cache/interface/[email protected]'. ========
======== Resolving module './AuthorityOptions' from '<redacted>/node_modules/@azure/msal-common/dist/authority/AuthorityFactory.d.ts'. ========
Resolution for module './AuthorityOptions' was found in cache from location '<redacted>/node_modules/@azure/msal-common/dist/authority'.
======== Module name './AuthorityOptions' was successfully resolved to '<redacted>/node_modules/@azure/msal-common/dist/authority/AuthorityOptions.d.ts' with Package ID '@azure/msal-common/dist/authority/[email protected]'. ========
======== Resolving module '../logger/Logger' from '<redacted>/node_modules/@azure/msal-common/dist/authority/AuthorityFactory.d.ts'. ========
Resolution for module '../logger/Logger' was found in cache from location '<redacted>/node_modules/@azure/msal-common/dist/authority'.
======== Module name '../logger/Logger' was successfully resolved to '<redacted>/node_modules/@azure/msal-common/dist/logger/Logger.d.ts' with Package ID '@azure/msal-common/dist/logger/[email protected]'. ========
======== Resolving module '../telemetry/performance/IPerformanceClient' from '<redacted>/node_modules/@azure/msal-common/dist/authority/AuthorityFactory.d.ts'. ========
Resolution for module '../telemetry/performance/IPerformanceClient' was found in cache from location '<redacted>/node_modules/@azure/msal-common/dist/authority'.
======== Module name '../telemetry/performance/IPerformanceClient' was successfully resolved to '<redacted>/node_modules/@azure/msal-common/dist/telemetry/performance/IPerformanceClient.d.ts' with Package ID '@azure/msal-common/dist/telemetry/performance/[email protected]'. ========
======== Resolving type reference directive 'node', containing file '<redacted>/node_modules/@azure/msal-common/dist/broker/nativeBroker/INativeBrokerPlugin.d.ts', root directory '<redacted>/node_modules/@types'. ========

The result of importing the node typings this way is that the typings from TypeScript's lib.dom.d.ts file for several DOM APIs such as setTimeout are overwritten with the Node versions, which have incompatible signatures. These are all declare global, since they are ambient - so they immediately infect the entire project being worked on.

And as this triple-slash reference is an explicit reference, it cannot be 'switched off' by using a tsconfig.json that specifies typings explicitly and disables implicit type imports. E.g.

{ "compilerOptions" : { "typings" : [] }}

The only viable and functioning workaround is to write a local, fake and empty version of the @types/node package and update package.json dependencies with that. e.g.

{
  // ...
  "devDependencies" : {
    // ...
    "@types/node" : "file:./packages/@types/node"
  }
}

Error Message

N/A

Msal Logs

N/A

MSAL Configuration

N/A

Relevant Code Snippets

import { PublicClientApplication } from "@azure/msal-browser";

// number is the actual correct return type for the DOM API as specified
// by lib.dom.d.ts
// However, the transitively included reference to @types/node will turn
// the return type into NodeJS.Timeout.
const timer:number = setTimeout(() => {}, 0);

Reproduction Steps

1. Incorporate @azure/msal-browser into a TS project.
2. Import anything from it, like PublicClientApplication
3. Put setTimeout into any of the code in your application meant for browsers.
4. Notice that it will use the wrongly typed and incompatible version from @types/node.
5. Perform tsc --traceResolution to see from where those types are imported.

Expected Behavior

@azure/msal-common should NOT contain triple-slash ///<reference types="node" /> declarations in its output typings. It is supposed to be a common library for all environments.

Identity Provider

Azure AD / MSA

Browsers Affected (Select all that apply)

Chrome, Firefox, Edge, Safari

Regression

No response

Source

External (Customer)

[sameerag]

@rjgotten Thanks for raising this. We have been making some improvements on this regard and will mark this as a todo on our end.

[ajhsu]

Hi @tnorling , our project is facing the same errors where the TS compiler complains after importing the @azure/msal-browser package:

1. Duplicated declarations

ERROR in node_modules/.pnpm/@[email protected]/node_modules/@types/node/globals.d.ts:33:13 - error TS2403: Subsequent variable declarations must have the same type.  Variable 'require' must be of type 'Require', but here has type 'NodeRequire'.

33 declare var require: NodeRequire;
               ~~~~~~~

  [internal_project_path]/require/2.3.2/require.d.ts:415:13
    415 declare var require: Require;
                    ~~~~~~~
    'require' was also declared here.

ERROR in node_modules/.pnpm/@[email protected]/node_modules/@types/node/module.d.ts:110:14 - error TS2300: Duplicate identifier 'Module'.

110     export = Module;
                 ~~~~~~

  [internal_project_path]/require/2.3.2/require.d.ts:38:11
    38  export = mod;
                 ~~~
    'Module' was also declared here.

2. Tons of type checking error due to the typings are overwritten with Node version, such as:

error TS2322: Type 'Timeout' is not assignable to type 'number'.

339         this.timeoutId = setTimeout(() => {
            ~~~~~~~~~~~~~~

The issue is blocking us from integrating with the @azure/msal-browser right now.

We tried to fake an empty "@types/node" as @rjgotten mentioned, to work around the issue but it doesn't seem to work well, and I believe it's kind of hacky and should not be an ideal solution for a production service; Thus, we are still looking forward to an appropriate patch to this issue. Thanks.

[rjgotten]

@ajhsu We tried to fake an empty "@types/node" as @rjgotten mentioned, to work around the issue but it doesn't seem to work well, and I believe it's kind of hacky and should not be an ideal solution for a production service; Thus, we are still looking forward to an appropriate patch to this issue. Thanks.

Oh, it's definitely not a very comfortable go-live solution. It's squarely in "if you got to; you've got to"-territory. But...

The alternative is typing around things manually everywhere and that's a bigger mess still. Not to mention the risks of the seductive, quote-unquote, 'solution' of "just use any" that starts playing its part at that point.

So I consider it the lesser of two evils.

[TylerLeonhardt]

@rjgotten can you elaborate on how you got the faking of @types/node to work? Did you update your package.json's devDeps with the fake @types/node or do you script updating MSAL.js's package.json?

We're hitting this when trying to adopt MSAL.js in https://vscode.dev. This is a pretty big blocker.

[rjgotten]

@TylerLeonhardt It's been a bit, but iirc I created a fake package and package.json on disk named @types/node and simply installed that as a filesystem type package within the affected project. And this was enough to ensure the real @types/node wasn't automagically requested and installed on the side.

From there I just needed to add the bare minimum of typings to play nice with the Node-isms the project in question actually did need (mainly related to legacy Webpack require syntax for context modules and the like).

---

Should you for whatever reason need to force it to be used for all dependencies transitively, you can probably do so via NPM overrides, which also frees you from the restriction of having to name the override @types/node and allows you to be more specific.

Off the top of my head:

"devDependencies": {
  "types-node-override" : "file:../packages/types-node-override",
},
"overrides": {
  "@types/node" : "$types-node-override"
}

---

Not sure if this will be a feasible way to move forward for your work on VSCode. But if it gets you going. Hey- that's great. Just ... as stated: caveat emptor on how stable this is as a workaround. My case is with a project that is still pre-production. I only applied this as a workaround because of that. I'm still banking on the issue being resolved properly before that project has to properly go live.

[TylerLeonhardt]

Got it working with this in my tsconfig.json:

"compilerOptions": {
  // ...
  // HACK to workaround: https://github.com/AzureAD/microsoft-authentication-library-for-js/issues/6269
  "typeRoots": ["packages/@types"],
  "types": ["node"]
 }

where the packages/@types/node folder had:

• package.json

{
     "name": "@types/node",
     "version": "16.11.21",
     "description": "OVERRIDE TypeScript definitions for Node.js",
     "homepage": "https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/node",
     "types": "index.d.ts"
}

• index.d.ts

/**
  * DO NOT USE. This package only exists in order to force MSAL.js to not
  * pull in @types/node because doing so polutes the global namespace and
  * causes many issues. See the link for more info.
  * 
  * @link https://github.com/AzureAD/microsoft-authentication-library-for-js/issues/6269
  * @deprecated so that there's a slash in the type.
  */
 declare module 'buffer' {
     global {
         interface Buffer extends Uint8Array {}
     }
 }

[WenyunZou]

I'm facing the same issue in v3(3.20.0). My workaround is to remove the triple-slash reference by pnpm patch.

-/// <reference types="node" />
 import { AccountInfo } from "../../account/AccountInfo";
 import { LoggerOptions } from "../../config/ClientConfiguration";
 import { NativeRequest } from "../../request/NativeRequest";
 import { NativeSignOutRequest } from "../../request/NativeSignOutRequest";
 import { AuthenticationResult } from "../../response/AuthenticationResult";
+interface Buffer extends Uint8Array {}
 export interface INativeBrokerPlugin {
     isBrokerAvailable: boolean;
     setLogger(loggerOptions: LoggerOptions): void;

Still looking forward to a fix.

[rjgotten]

@WenyunZou I'm facing the same issue in v3(3.20.0). My workaround is to remove the triple-slash reference by pnpm patch.

-/// <reference types="node" />
 import { AccountInfo } from "../../account/AccountInfo";
 import { LoggerOptions } from "../../config/ClientConfiguration";
 import { NativeRequest } from "../../request/NativeRequest";
 import { NativeSignOutRequest } from "../../request/NativeSignOutRequest";
 import { AuthenticationResult } from "../../response/AuthenticationResult";
+interface Buffer extends Uint8Array {}
 export interface INativeBrokerPlugin {
     isBrokerAvailable: boolean;
     setLogger(loggerOptions: LoggerOptions): void;

Still looking forward to a fix.

This approach would also work in the interim. Nice suggestion. Think I'm going to suggest switching to that on my end as well. What you could also try as a hot-patch, and what should afaik be the actual fix that should be applied by the library maintainers if they just want a quick-fix, is to use:

-/// <reference types="node" />
+import { Buffer } from "node:buffer";
 import { AccountInfo } from "../../account/AccountInfo";
 import { LoggerOptions } from "../../config/ClientConfiguration";
 import { NativeRequest } from "../../request/NativeRequest";
 import { NativeSignOutRequest } from "../../request/NativeSignOutRequest";
 import { AuthenticationResult } from "../../response/AuthenticationResult";
 export interface INativeBrokerPlugin {
     isBrokerAvailable: boolean;
     setLogger(loggerOptions: LoggerOptions): void;

That should import the actual Node.js Buffer type, iirc without bringing in all the ambient global types.