diff --git a/changelog.d/11044-ws-native-facade-reexport.md b/changelog.d/11044-ws-native-facade-reexport.md new file mode 100644 index 0000000000..e3630ef248 --- /dev/null +++ b/changelog.d/11044-ws-native-facade-reexport.md @@ -0,0 +1,13 @@ +Named re-exports of a Perry-native npm package (not a Node core builtin -- +`ws`, `ioredis`, `mysql2`, ...) through a local facade module now compile and +link. `export { X } from ""` previously fell through to the +generic re-export path, which has no compiled source module to follow for a +natively-intercepted package; codegen then expected a local function body for +the forwarded name that was never emitted, and referencing the binding as a +value (e.g. inside a closure) link-failed on an undefined +`__perry_wrap_perry_fn___` symbol. The synthetic-import +treatment that #10802/#10867 added for Node builtin re-exports is now applied +to any recognized Perry-native module source, not only `is_node_core_module` +ones. This unblocks ethers' `src.ts/providers/ws.ts` (`export { WebSocket } +from "ws";`), which `provider-websocket.ts` imports under a renamed local +binding and references inside a closure (#11044). diff --git a/crates/perry-codegen/src/codegen/mod.rs b/crates/perry-codegen/src/codegen/mod.rs index fc0f336415..71247b99aa 100644 --- a/crates/perry-codegen/src/codegen/mod.rs +++ b/crates/perry-codegen/src/codegen/mod.rs @@ -3631,6 +3631,12 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> .iter() .filter_map(|e| match e { perry_hir::Export::Named { local, exported } + // #11044: matches the broadened getter-emission gate in + // artifacts.rs — `import.is_native` alone, not restricted + // to node-core builtins, since non-core Perry-native + // packages (ws, ioredis, ...) get the same synthetic + // Import+Export pair from module_decl.rs's re-export + // handling and must skip this dead-stub arm too. if !hir.imports.iter().any(|import| { import.is_native && import.specifiers.iter().any(|specifier| { diff --git a/crates/perry-hir/src/lower/module_decl.rs b/crates/perry-hir/src/lower/module_decl.rs index 2eba8be1c8..2a32f93ff3 100644 --- a/crates/perry-hir/src/lower/module_decl.rs +++ b/crates/perry-hir/src/lower/module_decl.rs @@ -1463,19 +1463,35 @@ pub(crate) fn lower_module_decl( // A native module has no compiled source module for the // driver to follow through a normal ReExport edge. Model // the forwarding binding as a synthetic named import so - // codegen can publish a live getter for the native ESM - // export cell. This covers Node builtins and bundled npm - // shims such as `ws`; leaving the latter as ReExport made - // consumers reference a closure-wrapper symbol that no - // source module could emit (#11044). The synthetic local - // is compiler-private: `export { x } from "node:m"` does - // not introduce `x` into this module's lexical scope. + // codegen can publish a live getter for the builtin ESM + // export cell. The synthetic local is compiler-private: + // `export { x } from "node:m"` does not introduce `x` + // into this module's lexical scope. + // Node core builtins are the only sources with a + // manifest complete enough to validate named-export + // existence (`module_has_public_named_export` reads + // the generated API manifest, which is exhaustive + // only for core modules). Other Perry-native npm + // packages (ws, ioredis, mysql2, ...) still need the + // same synthetic-import treatment below — #11044: + // ethers' `ws.ts` does `export { WebSocket } from + // "ws"`, and without this a facade re-export of a + // non-core native package fell through to the + // generic `Export::ReExport` arm below, which has + // no compiled source module to follow either — so + // codegen expected a local function body that was + // never emitted, and the link failed on + // `__perry_wrap_perry_fn___`. let native_source = canonicalize_native_import_source(&source); if is_native_module(&native_source) { - if !perry_api_manifest::module_has_public_named_export( - &native_source, - &local, - ) { + let is_node_core = + perry_api_manifest::is_node_core_module(&native_source); + if is_node_core + && !perry_api_manifest::module_has_public_named_export( + &native_source, + &local, + ) + { crate::lower_bail!( named.span, "The requested module '{}' does not provide an export named '{}'", diff --git a/crates/perry-hir/tests/node_named_export_hygiene.rs b/crates/perry-hir/tests/node_named_export_hygiene.rs index 059f01d102..b79e712c27 100644 --- a/crates/perry-hir/tests/node_named_export_hygiene.rs +++ b/crates/perry-hir/tests/node_named_export_hygiene.rs @@ -224,6 +224,55 @@ fn bundled_package_named_reexports_lower_to_synthetic_native_imports() { })); } +/// #11044 follow-up: `module_has_public_named_export` reads the generated +/// `perry-api-manifest` API_MANIFEST, which only carries rows for the +/// exports someone has bothered to document per native package — it is not +/// exhaustive for anything but node-core builtins (see its own doc comment). +/// `ws` happens to have a manifest row for the one name these tests +/// re-export (`class("ws", "WebSocket")`), so it cannot tell a node-core- +/// scoped existence check apart from an unconditional one. `bcrypt` IS a +/// recognized `NATIVE_MODULES` entry (a real bundled native wrapper, not a +/// Node builtin) with manifest rows for `hash`/`compare` only — real +/// `bcrypt` also exports `genSalt`, `hashSync`, `compareSync`, `getRounds`, +/// none of which are in the manifest at all, so +/// `module_has_public_named_export("bcrypt", "genSalt")` is `false`. Before +/// this fix, `is_native_module` gated straight into that existence check for +/// every native module (not just node-core ones), so this exact re-export +/// hit `lower_bail!` and failed to compile even though `bcrypt.genSalt` is a +/// real export. Scoping the existence check to `is_node_core_module` — this +/// PR's only remaining delta from what `origin/main`'s independent #11044 +/// fix (b8c2457e4) already landed — skips that check for `bcrypt` and falls +/// through to the same permissive synthetic-import treatment `ws` gets. +#[test] +fn native_npm_package_export_missing_from_manifest_reexports_lower_to_synthetic_import() { + let module = lower_result(r#"export { genSalt } from "bcrypt";"#).expect( + "bcrypt.genSalt is a real export of the bcrypt package but has no \ + perry-api-manifest row (only hash/compare are documented); its \ + existence cannot be checked from the manifest, so lowering must \ + fall back to the permissive synthetic-import treatment instead of \ + lower_bail!-ing on a false negative", + ); + + assert!(module.imports.iter().any(|import| { + import.is_native + && import.source == "bcrypt" + && matches!( + import.specifiers.as_slice(), + [perry_hir::ImportSpecifier::Named { imported, local }] + if imported == "genSalt" + && local.starts_with("__perry_builtin_reexport_") + ) + })); + assert!(module.exports.iter().any(|export| { + matches!( + export, + perry_hir::Export::Named { local, exported } + if local.starts_with("__perry_builtin_reexport_") + && exported == "genSalt" + ) + })); +} + #[test] fn invalid_node_named_reexports_are_rejected() { let error = lower_result(r#"export { definitelyMissing } from "node:crypto";"#) diff --git a/crates/perry/tests/source_graph_export_regressions.rs b/crates/perry/tests/source_graph_export_regressions.rs index 6398586719..aa4fdad25f 100644 --- a/crates/perry/tests/source_graph_export_regressions.rs +++ b/crates/perry/tests/source_graph_export_regressions.rs @@ -978,3 +978,5 @@ mod issue_10417; mod issue_10432; #[path = "source_graph_export_regressions/issue_10758.rs"] mod issue_10758; +#[path = "source_graph_export_regressions/issue_11044.rs"] +mod issue_11044; diff --git a/crates/perry/tests/source_graph_export_regressions/issue_11044.rs b/crates/perry/tests/source_graph_export_regressions/issue_11044.rs new file mode 100644 index 0000000000..b9aeb3fc38 --- /dev/null +++ b/crates/perry/tests/source_graph_export_regressions/issue_11044.rs @@ -0,0 +1,45 @@ +use super::{compile_and_run, write}; + +// #11044: the node-builtin re-export fix (#10432/#10802/#10867) only +// special-cased `perry_api_manifest::is_node_core_module` sources. A local +// facade module re-exporting a named binding from a Perry-native NPM +// package that is NOT a node core builtin -- `ws`, same shape as `ioredis`, +// `mysql2`, ... -- fell through to the generic `Export::ReExport` arm, which +// has no compiled source module to follow either. Codegen then expected a +// local function body for the forwarded name that was never emitted, and +// referencing it as a value inside a closure (constructing it dynamically) +// link-failed on an undefined `__perry_wrap_perry_fn___` +// symbol. +// +// This is exactly the shape of ethers' `src.ts/providers/ws.ts` (`export { +// WebSocket } from "ws";`), imported under a renamed local binding by +// `provider-websocket.ts` and referenced inside +// `this.#connect = () => { return new _WebSocket(url); }`. +#[test] +fn native_npm_package_reexports_survive_local_facade_modules() { + let dir = tempfile::tempdir().expect("tempdir"); + write( + dir.path(), + "ws_facade.ts", + "export { WebSocket } from 'ws';\n", + ); + write( + dir.path(), + "main.ts", + "import { WebSocket as _WebSocket } from './ws_facade';\n\ + class Connector {\n\ + \x20 #connect: () => any;\n\ + \x20 constructor(url: string) {\n\ + \x20 this.#connect = () => { return new _WebSocket(url); };\n\ + \x20 }\n\ + \x20 hasConnector(): boolean { return typeof this.#connect === 'function'; }\n\ + }\n\ + const conn = new Connector('ws://127.0.0.1:1/');\n\ + console.log('hasConnector:', conn.hasConnector());\n", + ); + + assert_eq!( + compile_and_run(dir.path(), "main.ts"), + "hasConnector: true\n" + ); +} diff --git a/test-files/_helpers/gap_11044_ws_reexport/ws_facade.ts b/test-files/_helpers/gap_11044_ws_reexport/ws_facade.ts new file mode 100644 index 0000000000..901ae65e53 --- /dev/null +++ b/test-files/_helpers/gap_11044_ws_reexport/ws_facade.ts @@ -0,0 +1,5 @@ +// Mirrors ethers' `src.ts/providers/ws.ts`: a one-line facade module that +// re-exports a named binding from a Perry-native npm package which is NOT a +// Node core builtin. See test_gap_11044_native_facade_reexport_construct.ts +// and issue #11044. +export { WebSocket } from "ws"; diff --git a/test-files/test_gap_11044_native_facade_reexport_construct.ts b/test-files/test_gap_11044_native_facade_reexport_construct.ts new file mode 100644 index 0000000000..e792d2a314 --- /dev/null +++ b/test-files/test_gap_11044_native_facade_reexport_construct.ts @@ -0,0 +1,40 @@ +// #11044: `export { X } from ""` (crypto, path, ...) was +// fixed by #10432/#10802/#10867 -- a local facade module re-exporting a +// builtin's named value now publishes a live getter instead of link-failing +// on a nonexistent local function body. This is the same defect one layer +// out: a facade re-exporting a named binding from a Perry-native NPM +// package that is not a Node builtin (here `ws`; the same shape applies to +// `ioredis`, `mysql2`, ...). ethers' `src.ts/providers/ws.ts` is exactly +// `export { WebSocket } from "ws";`, imported (renamed) by +// `provider-websocket.ts` and referenced inside a closure -- which used to +// undefined-reference-fail the link on +// `__perry_wrap_perry_fn_..._ws_ts__WebSocket`. +// +// This program never opens a socket: constructing `Connector` only captures +// the reference-taking closure, matching the original ethers repro, which +// reaches the link failure from the closure alone (nothing calls `.open()`). +// Node cannot run this file (`ws` is a real npm package, not vendored here, +// and no node core module aliases it) -- same as the pre-existing sibling +// `test_gap_turnloop_ws_client.ts`, which hits the identical +// `ERR_MODULE_NOT_FOUND` under `node --experimental-strip-types`. Both are +// Perry-only correctness checks: real npm-`ws` behavior is covered by the +// hand-rolled-server gap tests instead. What this file exercises -- and +// what regressed to a link failure -- is purely the facade's cross-module +// symbol resolution, so a deterministic non-network assertion is the right +// shape here. +import { WebSocket as _WebSocket } from "./_helpers/gap_11044_ws_reexport/ws_facade.ts"; + +class Connector { + #connect: () => any; + constructor(url: string) { + this.#connect = () => { + return new _WebSocket(url); + }; + } + hasConnector(): boolean { + return typeof this.#connect === "function"; + } +} + +const conn = new Connector("ws://127.0.0.1:1/"); +console.log("hasConnector:", conn.hasConnector());