@azure/msal-browser transitively includes @types/node via @azure/msal-common and breaks typings #6269
Comments
rjgotten
added
bug-unconfirmed
microsoft-github-policy-service
bot
added
the
Needs: Attention 👋
github-actions
bot
added
msal-browser
sameerag
added
bug
microsoft-github-policy-service
bot
removed
question
|
@rjgotten Thanks for raising this. We have been making some improvements on this regard and will mark this as a todo on our end. |
|
Hi @tnorling , our project is facing the same errors where the TS compiler complains after importing the @azure/msal-browser package:
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.
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. |
Oh, it's definitely not a very comfortable go-live solution. The alternative is typing around things manually everywhere and that's a bigger mess still. So I consider it the lesser of two evils. |
|
@rjgotten can you elaborate on how you got the faking of We're hitting this when trying to adopt MSAL.js in https://vscode.dev. This is a pretty big blocker. |
|
@TylerLeonhardt 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 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 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. |
|
Got it working with this in my "compilerOptions": {
// ...
// HACK to workaround: https://github.com/AzureAD/microsoft-authentication-library-for-js/issues/6269
"typeRoots": ["packages/@types"],
"types": ["node"]
}where the
{
"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"
}
/**
* 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 {}
}
} |
|
I'm facing the same issue in v3(3.20.0). My workaround is to remove the triple-slash reference by pnpm patch. 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 |
Type resolution is broken for node16 resolution type due to several issues, this PR: - Updates all relative imports to include .js file extension, as required by node16 resolution - Includes type declaration files from lib folder in package publish - Adds package.json file to `lib` folder to indicate contents are commonjs - Updates package exports field to point to the appropriate type declaration files for ESM or CJS - Adds browser and node subpaths to msal-common export to separate node-only and browser-only features Fixes #6781 #6487 #6269 --------- Co-authored-by: Hector Morales <[email protected]>
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
PublicClientApplicationfrom@azure/msal-browserthis will lead to an import chain that will go through@azure/msal-browser/dist/index.d.tsto several dependencies from@azure/msal-common- which will be imported through@azure/msal-common/dist/index.d.tsThe latter will export
INativeBrokerPlugin:which, contains a triple-slash import directive of the
@types/nodepackage:This was verified via
tsc --traceResolution:The result of importing the node typings this way is that the typings from TypeScript's
lib.dom.d.tsfile for several DOM APIs such assetTimeoutare overwritten with the Node versions, which have incompatible signatures. These are alldeclare 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/nodepackage and updatepackage.jsondependencies with that. e.g.{ // ... "devDependencies" : { // ... "@types/node" : "file:./packages/@types/node" } }Error Message
N/A
Msal Logs
N/A
MSAL Configuration
Relevant Code Snippets
Reproduction Steps
@azure/msal-browserinto a TS project.PublicClientApplicationsetTimeoutinto any of the code in your application meant for browsers.@types/node.tsc --traceResolutionto see from where those types are imported.Expected Behavior
@azure/msal-commonshould 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)
The text was updated successfully, but these errors were encountered: