refactor(module)!: reject missing bare import() specifiers instead of placeholding

The optional-module placeholder now serves the require() path only.
ESM has a standards answer that CommonJS lacks — a dynamic-import failure
is a catchable rejection, so 'try { await import(x) } catch {}' covers
optional dependencies at the call site — and in a dev session a bare
specifier the import map doesn't cover is a configuration bug that the
placeholder was converting into a lazily-throwing proxy instead of a
loud resolver failure.

IsLikelyOptionalModule loses its header declaration and becomes static
to ModuleInternal.mm with its require()-only role documented. The specs
in NodeBuiltinsAndOptionalModulesTests.mjs pin the new contract: any
missing bare specifier rejects with 'Cannot find module', package-shaped
or not. The require()-side restore stays pending review, per
docs/knowledge/optional-module-placeholder.md.

If real-world dev-session testing surfaces a case that genuinely needs
the import()-side placeholder (a static import of an optional package in
a file-based ESM app), this commit is the isolated revert target.

Author: NathanWalker

NativeScript/runtime/ModuleInternal.h

@@ -6,14 +6,6 @@
 
 namespace tns {
 
-// Heuristic: does this bare specifier look like an npm-style package name
-// (as opposed to an explicit, extension-qualified file reference)? Shared by
-// both the CommonJS require() path (ModuleInternal.mm) and the ESM
-// import()/resolver path (ModuleInternalCallbacks.mm) so the two can't drift
-// out of sync with each other. See the definition in ModuleInternal.mm for
-// the full rationale.
-bool IsLikelyOptionalModule(const std::string& moduleName);
-
 class ModuleInternal {
  public:
   ModuleInternal(v8::Local<v8::Context> context);

NativeScript/runtime/ModuleInternal.mm

@@ -19,11 +19,11 @@
 
 namespace tns {
 
-// Shared by both the require() path (this file) and the import()/ESM resolver path
-// (ModuleInternalCallbacks.mm) — see the declaration in ModuleInternal.h. Keeping a single
-// definition means a boundary fix like the extension carve-out below automatically applies
-// to both call sites instead of only the one that happened to get touched.
-bool IsLikelyOptionalModule(const std::string& moduleName) {
+// require()-path policy only: import() rejects a missing bare specifier outright
+// (ESM optionality is `try { await import(x) } catch {}` at the call site, and in
+// dev sessions a bare specifier the import map doesn't cover is a config bug that
+// must fail loudly — see docs/knowledge/hmr-simplification-pass.md §2).
+static bool IsLikelyOptionalModule(const std::string& moduleName) {
   // Node built-ins are handled by their own dedicated resolution path; never treat them as
   // an optional external module.
   if (moduleName.rfind("node:", 0) == 0) {

NativeScript/runtime/ModuleInternalCallbacks.mm

@@ -28,8 +28,6 @@
 
 namespace tns {
 
-// IsLikelyOptionalModule is shared with the require() path — declared in
-// ModuleInternal.h, defined in ModuleInternal.mm.
 // Helper function to check if a module name is a Node.js built-in module
 static bool IsNodeBuiltinModule(const std::string& moduleName) {
   return moduleName.rfind("node:", 0) == 0;
@@ -2333,44 +2331,6 @@ static bool IsDocumentsPath(const std::string& path) {
       std::string msg = "Cannot find module '" + spec + "' (failed to create in-memory polyfill)";
       isolate->ThrowException(v8::Exception::Error(tns::ToV8String(isolate, msg)));
       return v8::MaybeLocal<v8::Module>();
-    } else if (IsLikelyOptionalModule(spec)) {
-      // Treat bare specifiers as optional modules with an in-memory placeholder ES module
-      // that throws on property access. This avoids bundle writes in iOS release builds.
-
-      std::string key = std::string("optional:") + spec;
-      auto itExisting = g_moduleRegistry.find(key);
-      if (itExisting != g_moduleRegistry.end()) {
-        v8::Local<v8::Module> existing = itExisting->second.Get(isolate);
-        if (!existing.IsEmpty() && existing->GetStatus() != v8::Module::kErrored) {
-          return v8::MaybeLocal<v8::Module>(existing);
-        }
-        RemoveModuleFromRegistry(key);
-      }
-
-      std::string placeholderContent = "const error = new Error(\"Module '" + spec +
-                                       "' is not available. This is an optional module.\");\n"
-                                       "const proxy = new Proxy({}, {\n"
-                                       "  get: function(target, prop) { throw error; },\n"
-                                       "  set: function(target, prop, value) { throw error; },\n"
-                                       "  has: function(target, prop) { return false; },\n"
-                                       "  ownKeys: function(target) { return []; },\n"
-                                       "  getPrototypeOf: function(target) { return null; }\n"
-                                       "});\n"
-                                       "export default proxy;\n";
-
-      v8::MaybeLocal<v8::Module> m =
-          CompileModuleForResolveRegisterOnly(isolate, context, placeholderContent, key);
-      if (!m.IsEmpty()) {
-        v8::Local<v8::Module> mod;
-        if (m.ToLocal(&mod)) {
-          return m;
-        }
-      }
-
-      std::string msg =
-          "Cannot find module '" + spec + "' (failed to create in-memory optional placeholder)";
-      isolate->ThrowException(v8::Exception::Error(tns::ToV8String(isolate, msg)));
-      return v8::MaybeLocal<v8::Module>();
     } else {
       std::string msg = "Cannot find module '" + spec + "' (tried " + absPath + ")";
       isolate->ThrowException(v8::Exception::Error(tns::ToV8String(isolate, msg)));

TestRunner/app/tests/NodeBuiltinsAndOptionalModulesTests.mjs

@@ -17,52 +17,34 @@ describe("Node built-in and optional module resolution", function () {
     expect(u.protocol).toBe("file:");
   });
 
-  it("creates an in-memory placeholder for likely-optional modules", async function () {
-    // Use a name that IsLikelyOptionalModule will treat as optional (no slashes, no extension).
-    const mod = await import("__ns_optional_test_module__");
-    const modAgain = await import("__ns_optional_test_module__");
-
-    expect(mod).toBeDefined();
-    expect(modAgain).toBe(mod);
-    expect(typeof mod.default).toBe("object");
-
-    let threw = false;
-    try {
-      // Any property access should throw according to the placeholder implementation.
-      // eslint-disable-next-line no-unused-expressions
-      mod.default.someProperty;
-    } catch (e) {
-      threw = true;
-    }
-    expect(threw).toBe(true);
-  });
-
-  // IsLikelyOptionalModule is shared between require() (CommonJS) and this ESM resolver
-  // (see ModuleInternal.h) so the boundary can't drift apart between the two paths. The
-  // require()-side cases live in shared/Require/index.js; these mirror them for import()
-  // to confirm the two stay in parity.
-  it("still creates a placeholder for a dotted-but-not-extension bare specifier", async function () {
-    // A dot that isn't a recognized file extension (e.g. an npm name like "lodash.debounce")
-    // must still be treated as optional, not as an explicit missing file.
-    const mod = await import("__ns_optional_test_module.dotted__");
-
-    expect(mod).toBeDefined();
-    expect(typeof mod.default).toBe("object");
+  // The optional-module placeholder is require()-path policy only (its specs live in
+  // shared/Require/index.js, pending review — docs/knowledge/optional-module-placeholder.md).
+  // import() deliberately diverges: a missing bare specifier rejects, package-shaped or not,
+  // so a dev-session import-map miss fails loudly instead of resolving to a lazily-throwing
+  // proxy. ESM callers that want optionality can `try { await import(x) } catch {}`.
+  it("rejects a missing bare specifier instead of resolving a placeholder", async function () {
+    const names = [
+      "__ns_optional_test_module__",
+      // A dot that isn't a recognized file extension (e.g. an npm name shaped like
+      // "lodash.debounce") gets no special treatment either.
+      "__ns_optional_test_module.dotted__",
+    ];
 
-    let threw = false;
-    try {
-      // eslint-disable-next-line no-unused-expressions
-      mod.default.someProperty;
-    } catch (e) {
-      threw = true;
+    for (const name of names) {
+      let error = null;
+      try {
+        await import(name);
+      } catch (e) {
+        error = e;
+      }
+      expect(error).not.toBe(null);
+      expect(String(error)).toContain("Cannot find module");
     }
-    expect(threw).toBe(true);
   });
 
   it("rejects immediately for a missing bare specifier that carries an explicit file extension", async function () {
-    // Mirrors the require()-side "video.js" case: a bare specifier with a recognized file
-    // extension is an explicit (here, missing) file reference, not an optional dependency,
-    // so import() must reject instead of resolving to a lazily-throwing placeholder.
+    // Extension-qualified names ("video.js") resolve through the filesystem-candidate
+    // walk rather than the bare-specifier path; pin that they reject the same way.
     const names = [
       "__ns_missing_import_test__.js",
       "__ns_missing_import_test__.json",

docs/knowledge/optional-module-placeholder.md

@@ -5,6 +5,17 @@ deliberately reinstates code that #412 removed. If the removal was intentional,
 this document is the argument to overturn — delete the machinery *and* the test
 and doc that still describe it, in one change.
 
+**Update (2026-07-29): the import()-side branch was removed again, this time
+deliberately.** The simplification pass
+([hmr-simplification-pass.md](hmr-simplification-pass.md) §2) split the
+question in two: on the ESM side a missing bare specifier now rejects
+outright — optionality is `try { await import(x) } catch {}` at the call
+site, and in dev sessions an import-map miss must fail loudly — while the
+require() side below, where the placeholder's lazy-throw contract is
+app-visible shipped behavior, stands exactly as this document describes and
+still needs Ed's call. The decision rule applied: remove first, and let a
+real-world reproduction argue it back in with evidence.
+
 ## What was restored
 
 `feat!: upgrade V8 to 14.9.207.39` (0e89474) deleted the whole optional-module