fix: support esm named imports
PR Checklist
Please check if your PR fulfills the following requirements:
- [X] The commit message follows our guidelines
- [ ] Tests for the changes have been added (for bug fixes / features)
- [ ] Docs have been added / updated (for bug fixes / features)
PR Type
What kind of change does this PR introduce?
- [ ] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, local variables)
- [X] Refactoring (no functional changes, no api changes)
- [ ] Adding Tests
- [ ] Build related changes
- [ ] CI related changes
- [ ] Documentation changes
- [ ] Other... Please describe:
What is the current behavior?
When using named imports in ESM you see the error saying that module.exports:
import { Client } from '@okta/okta-sdk-nodejs';
^^^^^^
SyntaxError: Named export 'Client' not found. The requested module '@okta/okta-sdk-nodejs' 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 '@okta/okta-sdk-nodejs';
const { Client } = pkg;
Though the default export mentioned in the message is technically possible, the need to do this isn't called out by tools like Typescript (more on this below)
Issue Number: N/A
What is the new behavior?
You can use named imports in ESM:
index.js
import { Client } from '@okta/okta-sdk-nodejs'
console.log('Client using named import', Client)
package.json
{
"type": "module",
"dependencies": {
"@okta/okta-sdk-nodejs": "7.1.1"
}
}
Running:
node ./index.js
Does this PR introduce a breaking change?
- [ ] Yes
- [X] No
Other information
Node making named exports via static analysis is part of a known issue in Typescript (i.e. the naming being available in Typescript but not in Node): https://www.typescriptlang.org/docs/handbook/modules/reference.html#interoperability-rules
Properties (other than default) of the CommonJS module’s module.exports may or may not be available as named imports to the ES module. Node.js attempts to make them available via static analysis. TypeScript cannot know from a declaration file whether that static analysis will succeed, and optimistically assumes it will. This limits TypeScript’s ability to catch named imports that may crash at runtime. See https://github.com/microsoft/TypeScript/issues/54018 for more details.
As called out in this typescript issue:
Node uses cjs-module-lexer to syntactically analyze CommonJS modules without executing them in order to turn module.exports properties into named exports that can be imported as named (or namespace) imports by ES modules. However, syntactic analysis has its limitations, and not all module.exports properties get detected on all modules. (In practice, it’s common for this to be all-or-nothing, as with the typescript package in its current state: it has no named exports available.)
Reviewers
@jaredperreault-okta or anyone else on the Okta team, is this something you would consider applying?
@jamesbuchan-okta or @oleksandrpravosudko-okta is anyone able to give a review?