Skip to content

Commit 23e4214

Browse files
robhoganmeta-codesync[bot]
authored andcommitted
Treat dynamic imports with rejection handlers as optional dependencies (#1697)
Summary: Pull Request resolved: #1697 Fixes: #1681 Extends the optional-dependency heuristic in `collectDependencies` to recognise dynamic imports that attach a rejection handler — either `.catch(handler)` or `.then(_, handler)` — anywhere in an unbroken promise chain rooted at the `import()` call. Today, `transformer.allowOptionalDependencies` only treats `require()` / `await import()` as optional when wrapped in a `try` block. Library code that relies on a chained `catch`, e.g. ```js import('node:diagnostics_channel') .then(dc => { ... }) .catch(() => { ... }); ``` (seen in `lru-cache` for instance: https://unpkg.com/lru-cache@11.3.5/dist/esm/diagnostics-channel.js ) still fails the build with an unresolvable-module error, even though the developer has clearly opted into a runtime fallback. This makes a pragmatic extension to `collectDependencies` to walk up the chain from the `import()` call and treat the dependency as optional if any chained call provides a rejection handler. The walk stops as soon as the chain is broken, keeping the heuristic local to the import. Changelog: ``` - **[Fix]**: Treat `import().catch()`, etc. as optional under `resolver.allowOptionalDependencies` ``` Reviewed By: huntie Differential Revision: D102145525 fbshipit-source-id: 4241ba24eb234e7aa42bbaa133b992477ef2327c
1 parent ccd9a57 commit 23e4214

2 files changed

Lines changed: 152 additions & 0 deletions

File tree

packages/metro/src/ModuleGraph/worker/__tests__/collectDependencies-test.js

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1605,6 +1605,91 @@ describe('optional dependencies', () => {
16051605
{name: 'foo', data: expect.not.objectContaining({isOptional: true})},
16061606
]);
16071607
});
1608+
1609+
describe('dynamic import with rejection handler', () => {
1610+
test('import().catch(handler) is optional', () => {
1611+
const ast = astFromCode(`
1612+
import('optional-async-a').catch(() => {});
1613+
`);
1614+
const {dependencies} = collectDependencies(ast, opts);
1615+
validateDependencies(dependencies, 2);
1616+
});
1617+
1618+
test('import().then(handler, onReject) is optional', () => {
1619+
const ast = astFromCode(`
1620+
import('optional-async-a').then(() => {}, () => {});
1621+
`);
1622+
const {dependencies} = collectDependencies(ast, opts);
1623+
validateDependencies(dependencies, 2);
1624+
});
1625+
1626+
test('import().then(...).then(...).catch(handler) is optional', () => {
1627+
const ast = astFromCode(`
1628+
import('optional-async-a')
1629+
.then(x => x)
1630+
.then(x => x)
1631+
.catch(() => {});
1632+
`);
1633+
const {dependencies} = collectDependencies(ast, opts);
1634+
validateDependencies(dependencies, 2);
1635+
});
1636+
1637+
test('await import().catch(handler) is optional', () => {
1638+
const ast = astFromCode(`
1639+
async function f() {
1640+
await import('optional-async-a').catch(() => {});
1641+
}
1642+
`);
1643+
const {dependencies} = collectDependencies(ast, opts);
1644+
validateDependencies(dependencies, 2);
1645+
});
1646+
1647+
test('try { await import() } catch {} is optional', () => {
1648+
const ast = astFromCode(`
1649+
async function f() {
1650+
try {
1651+
await import('optional-async-a');
1652+
} catch (e) {}
1653+
}
1654+
`);
1655+
const {dependencies} = collectDependencies(ast, opts);
1656+
validateDependencies(dependencies, 2);
1657+
});
1658+
1659+
test('import().then(handler) without onReject is not optional', () => {
1660+
const ast = astFromCode(`
1661+
import('not-optional-async-a').then(() => {});
1662+
`);
1663+
const {dependencies} = collectDependencies(ast, opts);
1664+
validateDependencies(dependencies, 2);
1665+
});
1666+
1667+
test('import().catch() with no handler argument is not optional', () => {
1668+
const ast = astFromCode(`
1669+
import('not-optional-async-a').catch();
1670+
`);
1671+
const {dependencies} = collectDependencies(ast, opts);
1672+
validateDependencies(dependencies, 2);
1673+
});
1674+
1675+
test('import().then(handler, null) is not optional (null/undefined onReject)', () => {
1676+
const ast = astFromCode(`
1677+
import('not-optional-async-a').then(() => {}, null);
1678+
import('not-optional-async-b').then(() => {}, undefined);
1679+
`);
1680+
const {dependencies} = collectDependencies(ast, opts);
1681+
validateDependencies(dependencies, 3);
1682+
});
1683+
1684+
test('import() detached from chain is not optional', () => {
1685+
const ast = astFromCode(`
1686+
const p = import('not-optional-async-a');
1687+
p.catch(() => {});
1688+
`);
1689+
const {dependencies} = collectDependencies(ast, opts);
1690+
validateDependencies(dependencies, 2);
1691+
});
1692+
});
16081693
});
16091694

16101695
test('uses the dependency transformer specified in the options to transform the dependency calls', () => {

packages/metro/src/ModuleGraph/worker/collectDependencies.js

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,15 @@ function isOptionalDependency(
630630
return false;
631631
}
632632

633+
// Treat dynamic imports as optional when a rejection handler is attached
634+
// close to the import call, e.g.
635+
// import('x').catch(handler)
636+
// import('x').then(handler, onReject)
637+
// import('x').then(...).catch(handler)
638+
if (isInPromiseChainWithRejectionHandler(path)) {
639+
return true;
640+
}
641+
633642
// Valid statement stack for single-level try-block: expressionStatement -> blockStatement -> tryStatement
634643
let sCount = 0;
635644
let p: ?(NodePath<> | NodePath<BabelNode>) = path;
@@ -652,6 +661,64 @@ function isOptionalDependency(
652661
return false;
653662
}
654663

664+
// Walk up a chain of `.then(...)` / `.catch(...)` member calls starting from
665+
// `path` (typically an `import()` CallExpression) and return true if any
666+
// chained call provides a rejection handler — either `.catch(handler)` or
667+
// `.then(_, handler)`. The chain must be unbroken: as soon as the parent is
668+
// not a member call applied to the previous expression, we stop. This keeps
669+
// the heuristic local to the import, matching the behaviour of the
670+
// try/catch heuristic above.
671+
function isInPromiseChainWithRejectionHandler(path: NodePath<>): boolean {
672+
let current: NodePath<> = path;
673+
while (current.parentPath != null) {
674+
const member = current.parentPath;
675+
if (
676+
member.node.type !== 'MemberExpression' ||
677+
member.node.object !== current.node ||
678+
member.node.computed ||
679+
member.node.property.type !== 'Identifier' ||
680+
member.parentPath == null
681+
) {
682+
return false;
683+
}
684+
const call = member.parentPath;
685+
if (
686+
call.node.type !== 'CallExpression' ||
687+
call.node.callee !== member.node
688+
) {
689+
return false;
690+
}
691+
const propertyName = member.node.property.name;
692+
const args = call.node.arguments;
693+
if (
694+
propertyName === 'catch' &&
695+
args.length >= 1 &&
696+
isNonNullishCallbackArg(args[0])
697+
) {
698+
return true;
699+
}
700+
if (
701+
propertyName === 'then' &&
702+
args.length >= 2 &&
703+
isNonNullishCallbackArg(args[1])
704+
) {
705+
return true;
706+
}
707+
current = call;
708+
}
709+
return false;
710+
}
711+
712+
function isNonNullishCallbackArg(arg: BabelNode): boolean {
713+
if (arg.type === 'NullLiteral') {
714+
return false;
715+
}
716+
if (arg.type === 'Identifier' && arg.name === 'undefined') {
717+
return false;
718+
}
719+
return true;
720+
}
721+
655722
function getModuleNameFromCallArgs(path: NodePath<CallExpression>): ?string {
656723
const args = path.get('arguments');
657724
if (!Array.isArray(args) || args.length !== 1) {

0 commit comments

Comments
 (0)