Skip to content

Commit

Permalink
Fix downstream ESM imports, fixes #6573 (#6600)
Browse files Browse the repository at this point in the history
Uses default import for importing keytar (a CJS module), which is really
the only foolproof way of importing CJS from an ESM context.

I confirmed by creating a simple package:
```json5
// msal-node-extensions/foo/package.json
{ "type": "module" }
```

```js
// msal-node-extensions/foo/index.js
import * as msal from "../dist/index.mjs";
msal;
```


Running `node foo/index.mjs` on `dev` gives the following error:

```
file:///.../microsoft-authentication-library-for-js/extensions/msal-node-extensions/dist/persistence/KeychainPersistence.mjs:3
import { setPassword, getPassword, deletePassword } from 'keytar';
                                   ^^^^^^^^^^^^^^
SyntaxError: Named export 'deletePassword' not found. The requested module 'keytar' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'keytar';
const { setPassword, getPassword, deletePassword } = pkg;

    at ModuleJob._instantiate (node:internal/modules/esm/module_job:124:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:190:5)

Node.js v18.17.1
```

Running the same command on this branch succeeds (no output, exit code
0).

Not sure of a good/reliable way to encode that in an automated test, but
I'd be happy to add one if anyone has any suggestions
  • Loading branch information
rzyns authored Oct 27, 2023
1 parent bfb5b4a commit ffe542b
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Fix downstream ESM imports, fixes #6573",
"packageName": "@azure/msal-node-extensions",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License.
*/

import { setPassword, getPassword, deletePassword } from "keytar";
import keytar from "keytar";
import { FilePersistence } from "./FilePersistence";
import { IPersistence } from "./IPersistence";
import { PersistenceError } from "../error/PersistenceError";
Expand Down Expand Up @@ -57,7 +57,11 @@ export class KeychainPersistence

public async save(contents: string): Promise<void> {
try {
await setPassword(this.serviceName, this.accountName, contents);
await keytar.setPassword(
this.serviceName,
this.accountName,
contents
);
} catch (err) {
if (isNodeError(err)) {
throw PersistenceError.createKeychainPersistenceError(
Expand All @@ -73,7 +77,7 @@ export class KeychainPersistence

public async load(): Promise<string | null> {
try {
return await getPassword(this.serviceName, this.accountName);
return await keytar.getPassword(this.serviceName, this.accountName);
} catch (err) {
if (isNodeError(err)) {
throw PersistenceError.createKeychainPersistenceError(
Expand All @@ -88,7 +92,10 @@ export class KeychainPersistence
public async delete(): Promise<boolean> {
try {
await this.filePersistence.delete();
return await deletePassword(this.serviceName, this.accountName);
return await keytar.deletePassword(
this.serviceName,
this.accountName
);
} catch (err) {
if (isNodeError(err)) {
throw PersistenceError.createKeychainPersistenceError(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License.
*/

import { setPassword, getPassword, deletePassword } from "keytar";
import keytar from "keytar";
import { FilePersistence } from "./FilePersistence";
import { IPersistence } from "./IPersistence";
import { PersistenceError } from "../error/PersistenceError";
Expand Down Expand Up @@ -58,7 +58,11 @@ export class LibSecretPersistence

public async save(contents: string): Promise<void> {
try {
await setPassword(this.serviceName, this.accountName, contents);
await keytar.setPassword(
this.serviceName,
this.accountName,
contents
);
} catch (err) {
if (isNodeError(err)) {
throw PersistenceError.createLibSecretError(err.message);
Expand All @@ -72,7 +76,7 @@ export class LibSecretPersistence

public async load(): Promise<string | null> {
try {
return await getPassword(this.serviceName, this.accountName);
return await keytar.getPassword(this.serviceName, this.accountName);
} catch (err) {
if (isNodeError(err)) {
throw PersistenceError.createLibSecretError(err.message);
Expand All @@ -85,7 +89,10 @@ export class LibSecretPersistence
public async delete(): Promise<boolean> {
try {
await this.filePersistence.delete();
return await deletePassword(this.serviceName, this.accountName);
return await keytar.deletePassword(
this.serviceName,
this.accountName
);
} catch (err) {
if (isNodeError(err)) {
throw PersistenceError.createLibSecretError(err.message);
Expand Down

0 comments on commit ffe542b

Please sign in to comment.