jvm-npm.js rewrite dropped Function-wrapper module isolation — const/let redeclaration crash across sibling requires

[djbclark]

TypeError: redeclaration of var X when two required files share a top-level const/let binding name

Symptom

Given a multi-file project like:

// lib/log.js
exports.append = function (line) { return line; };

// lib/a.js
const log = require("./log.js");
exports.tag = "a";

// lib/b.js
const log = require("./log.js");
exports.tag = "b";

// main.js
const a = require("./lib/a.js");
const b = require("./lib/b.js");

Running main.js crashes the instant the second file loads:

org.mozilla.javascript.EcmaError: TypeError: redeclaration of var log. (file:///android_asset/modules/jvm-npm.js#67)
	at org.mozilla.javascript.ScriptRuntime.constructError(...)
	at org.mozilla.javascript.ScriptableObject.redefineProperty(...)
	at org.mozilla.javascript.ScriptRuntime.initScript(...)
	at org.mozilla.javascript.Interpreter$CallFrame.initializeArgs(...)

Reproducible on-device (confirmed on a Pixel 7a — full incident: djbclark/stayturgid#34), independent of const/let/var, and independent of which file is the entry script. Because any real multi-file project routinely shares common dependency names (log, config, notify, etc.) across files, this affects every file that reuses a common local binding name, not just one.

What I believe is the root cause

app/src/main/assets/modules/jvm-npm.js in this repo has been substantially rewritten from upstream nodyn/jvm-npm (confirmed identical between SuperMonster003/AutoJs6 and this fork's copy — this isn't fork-specific). The rewrite changed how Module._load actually loads a required file's code:

Upstream nodyn/jvm-npm (Module._load):

var body = readFile(module.filename, module.core);
var func = new Function('exports', 'module', 'require', '__filename', '__dirname', body);
func.apply(module, [module.exports, module, module.require, module.filename, dir]);

Each required file's source becomes the body of a freshly-constructed Function, invoked with exports/module/require/etc. as its formal parameters. This gives genuine, JS-spec-guaranteed, function-local variable scoping — two different modules' top-level const log = ... can never collide, because each is scoped to a separate function call, full stop. (See also upstream issue nodyn/jvm-npm#37, which documents this exact mechanism and a portability caveat with older Rhino — relevant context, though not the same bug.)

This repo's rewritten jvm-npm.js (_.Module._load):

_load(file) {
    return NativeRequire.require(file);
},

The Function-wrapper mechanism has been removed entirely. Loading now delegates to NativeRequire.require(file) — i.e., whatever require implementation the host had installed before jvm-npm.js overwrote the global require binding (in this codebase, Rhino's own official org.mozilla.javascript.commonjs.module.Require, installed via RhinoJavaScriptEngine.initRequireBuilder() → RequireBuilder().setSandboxed(true).createRequire(...)). That class uses a fundamentally different isolation mechanism — Scriptable scope objects / prototype chains — not per-call function-local scoping.

I could not fully pin down (via several hours of controlled testing against this exact bundled org-mozilla-rhino-2_0_0-SNAPSHOT.jar, both via the bare Rhino shell and by mimicking initRequireBuilder's setup) the precise internal condition under which that scope-object-based isolation fails for const/let specifically — several plausible configurations I constructed did NOT reproduce the crash standalone, suggesting the failure also depends on some detail of this codebase's exact Kotlin-level wiring (AssetAndUrlModuleSourceProvider's dual asset+filesystem resolution, and/or the specific order of initRequireBuilder()/initPrologue()/initEpilogue()) that I wasn't able to fully replicate outside the real app. Full writeup of what I tried: https://github.com/djbclark/autojs6-typescript/blob/main/examples/broken/01-redeclaration/README.md

Regardless of that remaining internal detail, the actionable finding is clear: removing jvm-npm's original Function-wrapper isolation removed a structural guarantee (collision-proof scoping) and replaced it with a mechanism that doesn't appear to provide the same guarantee for ES6 const/let — syntax that didn't exist when either jvm-npm or (likely) this Rhino fork's commonjs module support were originally designed, and that any TypeScript project compiles to by default.

Suggested fix

Either restore genuine per-module Function-wrapper isolation in Module._load (matching upstream jvm-npm's original design), or otherwise verify/ensure the Scriptable-based delegation path actually provides equivalent guaranteed isolation for const/let-declared top-level bindings across sibling required files.

Related, not duplicate

• 无法模块化开发 #340 ("无法模块化开发" — module not found) and 悬浮窗停止脚本运行，外置配置文件无法加载。 #360 (require() outside a module) are different bugs in the same jvm-npm.js subsystem, not this one.
• Advisory: TypeScript-for-AutoJs6/Rhino gotchas found + a fork-agnostic verification toolkit #563 (my own earlier general advisory about a TypeScript verification toolkit for this engine) links here for the focused write-up.

Reference

• Minimal repro + local investigation: djbclark/autojs6-typescript, examples/broken/01-redeclaration/
• Original incident: djbclark/stayturgid#34
• Cross-filed on jvm-npm's own tracker (informational — the mechanism jvm-npm's own upstream design relies on for isolation was removed by this rewrite): (link added after filing)

[djbclark]

Cross-filed on jvm-npm's own tracker for a design-intent question about whether Module._load's host-delegation path was ever meant to substitute for the Function-wrapper isolation for regular file modules: nodyn/jvm-npm#43

[djbclark]

Correction/update: I retracted the jvm-npm.js cross-filing (nodyn/jvm-npm#43) — it wasn't warranted. Checked upstream jvm-npm's own source directly rather than treating this as an open design question:

• Module._load (loads every successfully-resolved file) always uses the new Function(exports, module, require, __filename, __dirname, body) wrapper — no exceptions, no delegation.
• NativeRequire.require is used in exactly one place upstream: inside Require()'s if (!file) branch, i.e. purely as a not-found/native-module fallback for ids that can't be resolved to a file at all.

So there's no ambiguity: upstream jvm-npm never used (or intended) NativeRequire.require as a substitute for Module._load's isolation on regular file modules. This repo's rewrite of jvm-npm.js made that substitution unilaterally — it's not an extension of jvm-npm's own design, and jvm-npm's maintainers have no design intent to clarify here. This is entirely this repo's own change to weigh in on.

[djbclark]

For context, this was found while building the toolkit mentioned in #563 (the general advisory) — cross-linking for anyone browsing either.