JavaScript: bindModule gives a recipe a local binding on either module system

[knutwannheden]

recipes-ui5 carries ~600 lines in src/amd.ts doing for sap.ui.define what maybeAddImport does for ES modules, and its MigrateDeprecatedUi5Api — 210 of the UI5 linter's 217 curated fixes — no-ops entirely on ESM sources. Adding an ESM lane there meant a second implementation of one idea in one module, plus the framework's.

The operation those recipes want is not "add an import". It is "give me a local binding for module M, creating one if needed" — and an AMD caller cannot supply the name, because the module may already be bound under another name, the conventional name may be taken, or binding may be impossible. AMD pairs the dependency array with the factory parameter list by position, and a parameter can only be appended at the end, so a block declaring a different number of dependencies than parameters cannot be extended without binding the new module against the wrong one.

const element = await bindModule(this, "sap/ui/core/Element");
if (element === undefined) return call;
return template`${raw(element)}.getElementById(${id})`.apply(call, this.cursor, {values: match});

That recipe is unchanged between the two module systems:

sap.ui.define(["sap/m/Button"], function (Button) { sap.ui.getCore().byId("p"); });
sap.ui.define(["sap/m/Button", "sap/ui/core/Element"], function (Button, Element) { Element.getElementById("p"); });

import Button from "sap/m/Button";                 import Button from "sap/m/Button";
                                                   import Element from "sap/ui/core/Element";
sap.ui.getCore().byId("p");                        Element.getElementById("p");

Alongside it: moduleBindings reports what a file binds and which lane it uses, isAmdBlock answers the node-scoped question, maybeRemoveImport gains an AMD lane, and removeNewlyUnusedAmdBindings drops dependencies a rewrite orphaned.

Why not a style flag on maybeAddImport

member and typeOnly are unsatisfiable where a factory parameter is the only binding form; the AMD lane can refuse and the ESM lane cannot; and answering costs a scan of cu.statements on one lane and a traversal of the factory body on the other, so one is synchronous and one cannot be. A flag would gate half the options on its own value. bindModule decides everything synchronously from the cursor and defers only the edit, so the execution model stays the one maybeAddImport established. Reach for it where a file might be AMD; a recipe that only meets ES modules should keep calling maybeAddImport, whose ESM behaviour is what bindModule's ESM lane delegates to.

What the positional pairing costs

Every edit keeps both lists index-aligned, because nothing in the LST enforces it and a mistake parses, prints plausibly, and binds later modules to wrong names. Review found several of exactly that shape, each verified by parsing, editing and printing rather than by reading: an arrow factory's trailing comma is an extra J.Empty rather than a TrailingComma marker, so the two factory forms disagree and appending produced (B,,D); a one-parameter arrow needs parenthesized set or prints B , D=> {}; a factory declaring fewer parameters than the block has dependencies took an appended parameter at the wrong index; and a destructuring declaration's names never reached the deconfliction pool, so const {Element} = window in a body emitted a duplicate binding.

Tests

test/javascript/amd.test.ts pins the mechanics against a hand-driven visitor, test/javascript/bind-module.test.ts pins the API — 58 between them, one per decision. The four deconfliction tests and the two refusal tests were kept separate because each dies to a different mutation; where two died to the same one-line revert they were merged. declaredNames' four branches — identifier, object pattern, nesting, rest element — are each mutation-verified.

Scope

add-import.ts changes by one word, exporting moduleNameOf. No AMD analogue of a side-effect import yet, so recipes-ui5 keeps withUnboundDependency; the removal sweep is AMD-only, so an orphaned import stays; and amdCallee does not reach maybeRemoveImport, whose signature takes no options. moduleSystem reports "none" for a file with no module syntax, because bindModule binding into a plain script would turn it into a module — the caller decides, since adding a first import to a file meant to be a module is ordinary and the two are indistinguishable.

Design is recorded in doc/adr/0013-javascript-module-bindings.md.

Downstream

Ported across five recipes-ui5 recipes: net −251 source lines, 252 tests green. MigrateDeprecatedUi5Api loses modulesNeeded, its bindings map and one of three visitors, dropping to a single traversal, and starts working on ESM sources with no ESM-specific code. Porting also removed two latent false-negative findings caused by the pre-scan those recipes used: no-globals reported every ES-module binding in a file without a sap.ui.define as an undeclared global, because a pre-scan that finds no block is indistinguishable from a block that binds nothing.

• Builds on maybeAddImport returns the name it bound, and templates resolve the modules they name #8680, now merged.

[knutwannheden]

I am still checking how we can unify this with maybeAddImport. I wasn't even aware of these AMD module dependencies until yesterday.