Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions changelog.d/11044-ws-native-facade-reexport.md
Original file line number Diff line number Diff line change
@@ -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 "<native-package>"` 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_<facade>__<name>` 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).
6 changes: 6 additions & 0 deletions crates/perry-codegen/src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3631,6 +3631,12 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result<Vec<u8>>
.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| {
Expand Down
38 changes: 27 additions & 11 deletions crates/perry-hir/src/lower/module_decl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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_<mod>__<name>`.
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 '{}'",
Expand Down
49 changes: 49 additions & 0 deletions crates/perry-hir/tests/node_named_export_hygiene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";"#)
Expand Down
2 changes: 2 additions & 0 deletions crates/perry/tests/source_graph_export_regressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
45 changes: 45 additions & 0 deletions crates/perry/tests/source_graph_export_regressions/issue_11044.rs
Original file line number Diff line number Diff line change
@@ -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_<facade>__<name>`
// 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"
);
}
5 changes: 5 additions & 0 deletions test-files/_helpers/gap_11044_ws_reexport/ws_facade.ts
Original file line number Diff line number Diff line change
@@ -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";
40 changes: 40 additions & 0 deletions test-files/test_gap_11044_native_facade_reexport_construct.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// #11044: `export { X } from "<node-core-builtin>"` (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());
Loading