Skip to content

cjs: a conditional require() of a target with no default export is never evaluated, in every conditional shape #10754

Description

@proggeramlug

Summary

On main (v0.5.1605, 91a566c8af), a top-level CommonJS require() in short-circuit, ternary, try, switch or loop-body position is never evaluated — the module does not load even when the branch is taken. Node loads it.

The if shape is correct. Only the other conditional positions are affected.

Reproduction

// sc.cjs
console.log('start');
const go = process.argv.includes('--go');
go && require('./a.cjs');          // a.cjs: console.log('a')
console.log('end');
$ node sc.cjs --go          $ ./sc --go        (perry 0.5.1605)
start                       start
a                           end
end                         ← 'a' never printed

Same for a ternary:

const v = go ? require('./b.cjs') : 0;
node:  start / b / end          perry: start / end

Combined fixture covering all five shapes — short-circuit, ternary, try block, switch case, loop body — with --go set:

node:  start a b c d e end
perry: start end

All five are dropped. Compiled with the release compiler at 91a566c8af, PERRY_RUNTIME_DIR pinned to the matching artifact set.

Why this is worse than the bug it replaced

The original defect (#10437) was that a conditional require was hoisted into an eager synthetic import, so the target initialized even when the branch was never taken — wrong order, but the module did load. #10674 fixed that by deferring instead of hoisting, and its if-shape handling is correct.

In these five positions the deferral appears to have no evaluation site, so the require is silently discarded. A program that conditionally loads a module now behaves as though the module does not exist, with no error. That is a silent wrong answer rather than a visible failure.

Relationship to #10285

#10285 is open and explicitly claims these shapes — its description names "a branch, ternary, short-circuit operand, logical assignment, try block, switch case or loop body". It merges cleanly against current main and adds cjs_wrap/deferred_requires.rs (254 lines) plus a 377-line integration suite, neither of which is on main.

It appears to have stalled because #10674 landed a different implementation of overlapping intent (train #10716, v0.5.1598) and #10285 was treated as superseded. It isn't: #10674 covers the if shape, #10285 claims all of them. Whether #10285 actually fixes these five is unverified here and is the next thing to check.

Not verified

Whether this is a regression introduced by #10674 or a pre-existing gap it did not cover — establishing that needs a compiler built before that train, which this report did not do.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions