fix(native): mark CJS require bindings to fix receiver-edge parity#1679
Conversation
…1671) const { X } = require('./path') creates a kind='function' shadow node in the importing file. The Rust emit_receiver_edge checks is_local_definition via imported_names.contains_key(name) — but CJS require bindings were not added to symbols.imports, so the name appeared to be a local non-class symbol and blocked the cross-file class lookup. Fix: - types.rs: add cjs_require: Option<bool> to Import struct - javascript.rs: set cjs_require=Some(true) on const { X } = require() imports - pipeline.rs: use empty target_file for CJS entries in collect_imported_names_for_file (so import-aware call-target lookup misses and falls through to shadow nodes, matching WASM behaviour) and skip CJS in resolve_pipeline_imports - import_edges.rs: skip CJS require imports in build_import_edges (no DB import edges) Also fix pre-existing biome lint/format issues in dart.ts, wasm-worker-entry.ts, dart.test.ts, and objc.test.ts. Fixes #1671 Impact: 2 functions changed, 4 affected
Greptile SummaryThis PR fixes a receiver-edge parity bug between the WASM and native engines:
Confidence Score: 5/5Safe to merge; the CJS binding fix is mechanically correct and the dataflow transaction improvement closes a real crash-window gap. The No files require special attention; the Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["const { X } = require('./path')"] --> B{Rust extractor}
B --> C["extract_destructured_bindings\n→ shadow node in definitions"]
B --> D["NEW: cjs_require = Some(true)\n→ Import added to symbols.imports"]
D --> E{collect_imported_names_for_file}
E --> F["CJS path: ImportedName\n{ name: 'X', file: '' }"]
F --> G{emit_receiver_edge}
G --> H["imported_names.contains_key('X') → true\nis_local_definition = false"]
H --> I["Cross-file class lookup via\nnodes_by_name proceeds ✓"]
D --> J{resolve_pipeline_imports}
J --> K["cjs_require=true → skip\nNo DB import edge created"]
D --> L{build_import_edges}
L --> M["cjs_require=true → skip\nNo spurious import edge in DB"]
style I fill:#22c55e,color:#fff
style K fill:#3b82f6,color:#fff
style M fill:#3b82f6,color:#fff
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["const { X } = require('./path')"] --> B{Rust extractor}
B --> C["extract_destructured_bindings\n→ shadow node in definitions"]
B --> D["NEW: cjs_require = Some(true)\n→ Import added to symbols.imports"]
D --> E{collect_imported_names_for_file}
E --> F["CJS path: ImportedName\n{ name: 'X', file: '' }"]
F --> G{emit_receiver_edge}
G --> H["imported_names.contains_key('X') → true\nis_local_definition = false"]
H --> I["Cross-file class lookup via\nnodes_by_name proceeds ✓"]
D --> J{resolve_pipeline_imports}
J --> K["cjs_require=true → skip\nNo DB import edge created"]
D --> L{build_import_edges}
L --> M["cjs_require=true → skip\nNo spurious import edge in DB"]
style I fill:#22c55e,color:#fff
style K fill:#3b82f6,color:#fff
style M fill:#3b82f6,color:#fff
Reviews (2): Last reviewed commit: "fix: resolve merge conflicts with main" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let names = collect_object_pattern_names(&name_n, source); | ||
| if !names.is_empty() { | ||
| let mut imp = Import::new( | ||
| mod_path, names, start_line(node), | ||
| ); | ||
| imp.cjs_require = Some(true); | ||
| symbols.imports.push(imp); | ||
| } |
There was a problem hiding this comment.
For const { Calculator: CalcAlias } = require('./calc'), collect_object_pattern_names returns "Calculator" (the key/export name, via child_by_field_name("key")), but extract_destructured_bindings creates a shadow node named "CalcAlias" (the value/local alias, via child_by_field_name("value")). At receiver-edge resolution time, effective_receiver is "CalcAlias" (from the type map), so imported_names.contains_key("CalcAlias") returns false and is_local_definition is still set to true — the same bug as before the fix.
The non-aliased case (const { Calculator } = require('./calc')) is handled correctly since both functions extract the same shorthand name. Aliased CJS destructuring is uncommon, but the gap is worth noting.
docs check acknowledged Impact: 1 functions changed, 2 affected
Codegraph Impact Analysis6 functions changed → 14 callers affected across 5 files
|
1 participant
Summary
const { X } = require('./path')creates akind='function'shadow node in the importing file, which caused the native engine'semit_receiver_edgeto treat it as a locally-defined non-class symbol and block the cross-file class fallback lookupsymbols.importsin the Rust extractor, soimported_names.contains_key(name)returned false for names likeCalculator, makingis_local_definition = truereceiveredge (frommain → Calculator) forcalc.compute()whencalcwas typed viaconst calc = new Calculator()Root cause: The WASM engine fixed this via
cjsRequireBindings(issue #1661), but the native engine had no equivalent.Fix:
types.rs: addcjs_require: Option<bool>toImportstructjavascript.rs: setcjs_require = Some(true)onconst { X } = require()imports so the receiver-edge resolver treats them as import artifactspipeline.rs: use emptytarget_filefor CJS entries incollect_imported_names_for_file(import-aware call-target lookup misses → falls through to shadow nodes, matching WASM behavior) and skip CJS inresolve_pipeline_importsimport_edges.rs: skip CJS require imports inbuild_import_edges(no spurious DB import edges)Test plan
npx vitest run tests/integration/build-parity.test.ts— all 8 tests pass (was failing before fix)npx vitest run tests/— 3283 tests pass, no regressionscargo test— 422 Rust tests passFixes #1671