Incorrect type definitions for msal-browser and msal-react

[charliematters]

Core Library

MSAL.js (@azure/msal-browser)

Core Library Version

3.6.0

Wrapper Library

MSAL React (@azure/msal-react)

Wrapper Library Version

2.0.8

Public or Confidential Client?

Public

Description

We're using tshy to bundle a library which uses msal-browser and msal-react, and it is failing for type reasons:

The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("@azure/msal-browser")' call instead.
  To convert this file to an ECMAScript module, change its file extension to '.mts' or create a local package.json file with `{ "type": "module" }`

The last time I asked the tshy author about this, they directed me to this page for checking type correctness, and it's flagging a few issues on both packages:

@azure/msal-browser

Problems
👺
Masquerading as ESM

Import resolved to an ESM type declaration file, but a CommonJS JavaScript file.

🥴
Internal resolution error

Import found in a type declaration file failed to resolve. Either this indicates that runtime resolution errors will occur, or (more likely) the types misrepresent the contents of the JavaScript files.

@azure/msal-react

Problems
⚠️
ESM (dynamic import only)

A require call resolved to an ESM JavaScript file, which is an error in Node and some bundlers. CommonJS consumers will need to use a dynamic import.

🥴
Internal resolution error

Import found in a type declaration file failed to resolve. Either this indicates that runtime resolution errors will occur, or (more likely) the types misrepresent the contents of the JavaScript files.

I notice that this issue is probably related: #6377

Error Message

The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("@azure/msal-browser")' call instead.
To convert this file to an ECMAScript module, change its file extension to '.mts' or create a local package.json file with { "type": "module" }

Msal Logs

No response

MSAL Configuration

{}

Relevant Code Snippets

-

Reproduction Steps

Expected Behavior

No type errors, or at least allow compilation using tshy (which uses tsc under the hood)

Identity Provider

Azure AD / MSA

Browsers Affected (Select all that apply)

Other

Regression

No response

Source

External (Customer)

[sameerag]

@charliematters is this still seen?

[charliematters]

It looks like that type-checking website is still reporting the same errors, yes

[koooge]

Hi there,
I encountered the same issue in @azure/msal-node@2.7.0. My project is commonjs using moduleResolution node16.

        "module": "node16",
        "moduleResolution": "node16",
        "esModuleInterop": true,

https://arethetypeswrong.github.io/?p=%40azure%2Fmsal-node%402.7.0

[klippx]

Problem

The problem is that you export this artefact ./dist/index.d.ts which contains e.g.

export type { UnauthenticatedTemplateProps } from "./components/UnauthenticatedTemplate";
export { useIsAuthenticated } from "./hooks/useIsAuthenticated";
export { useMsalAuthentication } from "./hooks/useMsalAuthentication";

The issue is not so bad, as it is just types and runtime things work anyway, but the issue can be for instance Typescript will not find the exports correctly and everything from your library will implicitly be any:

Which can be a problem if you are a very strict project and do not allow any - for instance, people who know what they are doing and can be bothered to configure eslint with typescript introspection to discover these issues automatically.

Fix

The exported artefacts should use absolute imports. In the example above, it should rather look like this:

export type { AuthenticatedTemplateProps } from "./components/AuthenticatedTemplate.d.ts";
export { useIsAuthenticated } from "./hooks/useIsAuthenticated.js";
export { useMsalAuthentication } from "./hooks/useMsalAuthentication.js";

Of course, it is not only this file but ALL files in the dist/**/* folder that needs to follow this convention.

E.g in ./dist/hooks/useMsalAuthentication.d.ts:

import { AccountIdentifiers } from "../types/AccountIdentifiers.d.ts"; // <-- .d.ts extension added

[klippx]

My workaround right now is to enumerate all files that directly or indirectly import from any msal/* library, turning off the eslint rules.

Of course, I still do not get any Typescript support while coding, only any, but at least the project compiles.

module.exports = {
  overrides: [
    // Any file that directly or indirectly import from '@azure/msal-browser' / '@azure/msal-react'
    // must unfortunately turn off typescript-eslint, since type exports are not working as required.
    {
      files: [
        'src/components/Authentication.tsx',
        'src/components/GraphiQLContainer/plugins/Collection/useCollectionOperations.tsx',
        'src/components/GraphiQLContainer/useCopyQuery.tsx',
        'src/components/MsalProvider.tsx',
        'src/components/SideNav/ActiveLink.tsx',
        'src/components/SideNav/BottomAction.tsx',
        'src/components/SideNav/BottomActions.tsx',
        'src/components/SideNav/SideNavView.tsx',
        'src/components/TopBar/CurrentUser.tsx',
        'src/config/authConfig.ts',
        'src/utils/hooks/useAccessToken.tsx',
        'src/utils/hooks/useActiveAccount.tsx',
        'src/utils/trpc.ts',
      ],
      rules: {
        '@typescript-eslint/no-unsafe-argument': 'off',
        '@typescript-eslint/no-unsafe-assignment': 'off',
        '@typescript-eslint/no-unsafe-call': 'off',
        '@typescript-eslint/no-unsafe-member-access': 'off',
        '@typescript-eslint/no-unsafe-return': 'off',
      },
    },
  ],
  // ...

[karlbohlmark]

Another workaround seems to be to change moduleResolution to node10, but we would love to see this fixed.

[juliangoacher]

I came across this problem when setting up a react-admin auth provider and had to do the following to fix:

• Set "module": "es2022" and "moduleResolution": "bundler" in ts settings
• Then pin the @azure/msal-browser package to v3.7.0, in order to fix subsequent type errors when using PublicClientApplication

Would be great if the issues described above could be fixed though.

[maorleger]

Seeing the same issue here in @azure/identity - because the type declarations are ESM, TypeScript will falsely complain about CommonJS requireing an ESM reference.

Please note that we use @azure/msal-node, @azure/msal-browser, (and @azure/msal-common indirectly) - I know this issue only refers to msal-browser and msal-react; however, please expand the scope to include @azure/msal-node as well 🙏

@sameerag are there any plans to fix the typings here? As a result, we're unable to dual-publish ESM/CJS package for @azure/identity without resorting to painful workarounds so I just wanted to check if y'all have a timeline for addressing this

https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FalseESM.md#common-causes
https://arethetypeswrong.github.io/?p=%40azure%2Fmsal-node%402.12.0

Many thanks!

[ethanshry]

Another way I have worked around the build error is by setting the "skipLibCheck": true, flag in my tsconfig.json- which is still a subpar solution, but maybe will unblock someone.