fix: #845 — cjs_wrap no longer corrupts const X = require('Y').Z; and module.exports.default#881
Merged
Merged
Conversation
…nd `module.exports.default`
Two parse-failure sub-bugs and one ergonomics miss surfaced by mysql2 and
pino under `perry.compilePackages`. All three produced byte offsets past
EOF of the original file, because the SWC error refers to the post-wrap
source while the message names the on-disk path.
mysql2 — TS1109 "Expression expected". The require-alias regex in
extract_require_aliases_with_ranges greedy-matched
`const EventEmitter = require('events').EventEmitter;` as
`const EventEmitter = require('events')` (`;?` stopped at `)`). The
blanking pass that strips inner `var X = require(...)` (so the hoisted-
class machinery's module-scope alias doesn't get shadowed) then replaced
the match with spaces, leaving `.EventEmitter;` dangling at column 0 of
the wrap output. Fix: require either `;` or end-of-line after `)` so
trailing `.X` member access disqualifies the match — those bind a
property, not the module object, so the alias-rename pass would be wrong
anyway. Whole-statement aliases (incl. same-line follow-on statements
like `var dep = require('./dep'); module.exports = dep.value;`) still
match.
pino — ExpectedIdent on `default`. pino.js ends with
`module.exports.default = pino` (ESM-interop convention), and
extract_exports_from_source surfaced `default` as a named export;
named_export_decls then emitted `export const default = _cjs.default;`,
which is invalid syntax. Fix: new is_js_reserved_word helper; push_unique
skips reserved-word names (same precedent as the existing __esModule
skip). `default` is the only common real-world case; the separate
default_export_decl emits `export default _cjs;` so nothing is lost. The
direct_named_reexport emission (`export { _req_N as NAME }`) is
intentionally unfiltered — `as default` / `as class` IS valid in export
clauses.
Ergonomics — new annotate_parse_error helper in collect_modules.rs that
fires only when cjs_wrap actually ran. It regex-pulls the `(lo..hi, …)`
span out of SWC's Debug-format, then appends a note explaining the wrap,
both byte sizes (wrap output vs on-disk), and a 5-line excerpt of the
wrap output around the offending offset with the failing line marked
`>>>`. Plus a `PERRY_DEBUG_CJS_WRAP=1` pointer. Pass-through for
non-wrapped sources.
Tests: four new unit tests in cjs_wrap::tests
(require_alias_extract_skips_trailing_member_access,
wrap_does_not_dangle_member_access_after_blanking,
extract_exports_skips_default_reserved_word, wrap_pino_shape_parses_cleanly).
End-to-end: real mysql2 / pino packages from local node_modules — pre-fix
produces the issue's exact two errors; post-fix pino compiles to an
executable and mysql2 advances past the parse step (hits a downstream
HIR bug at `_connectionId++` — separate, out of scope).
Closes #845. Refs #804 (drizzle e2e — mysql2 was the first parse blocker),
#793 (compat roadmap).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #845. Two parse-failure sub-bugs + one ergonomics miss under
perry.compilePackages, both surfaced via real mysql2 / pino packages.extract_require_aliases_with_rangesgreedy-matchedconst EventEmitter = require('events').EventEmitter;asconst EventEmitter = require('events')(the regex's;?happily stopped at)). The hoisted-class blanking pass then replaced that match with spaces, leaving.EventEmitter;dangling at column 0 of the wrap output. Fix: require;or end-of-line after)— trailing.Xmember access disqualifies the match.module.exports.default = pino(ESM-interop) flowed intonamed_export_declsasexport const default = _cjs.default;, which is invalid syntax. Fix: newis_js_reserved_wordhelper;push_uniqueskips reserved-word names (same precedent as the existing__esModuleskip). The separatedefault_export_declstill covers default exports.direct_named_reexports(export { _req_N as NAME }) stays unfiltered —as defaultIS valid in export clauses.annotate_parse_errorhelper fires only when cjs_wrap actually ran. Appends a note explaining the wrap, both byte sizes (wrap output vs on-disk), how to dump it (PERRY_DEBUG_CJS_WRAP=1), and a 5-line excerpt with the offending line marked>>>. Pass-through for non-wrapped sources.Example post-fix message for a deliberately-broken
compilePackagestarget:Test plan
cargo test --release -p perry --bin perry commands::compile::cjs_wrap— 41/41 green (4 new tests: regex tightening, full wrap→parse for mysql2 shape, reserved-word filter, full wrap→parse for pino shape).cargo test --release --workspaceminus cross-host UI crates — 836/0 green.cargo test --release -p perry-runtime --lib -- --test-threads=1— 255/0 green (a parallel-run TUI test is a pre-existing shared-SLOTSflake, unrelated)./tmp/run_gap_tests.sh— 35/36, unchanged from baseline (lone fail is the pre-existingtest_gap_console_methods)./tmp/repro845with mysql2 from localnode_modules: pre-fix produces the exact issue error(35073..35074, TS1109)at offset past EOF; post-fix advances past the parse step (hits a separate downstream HIR bug at_connectionId++— out of scope here)./tmp/repro845with pino: pre-fix produces(8282..8289, ExpectedIdent)past EOF; post-fix compiles to a 2.5 MB executable./tmp/repro845bwith a deliberately-brokencompilePackagestarget: produces the annotated error message shown above.Per CLAUDE.md's external-contributor convention, this PR does NOT touch
Cargo.toml[workspace.package].version,CLAUDE.md's**Current Version:**line, orCHANGELOG.md. The maintainer bumps + writes the changelog at merge time.Refs #804 (drizzle e2e — mysql2 was the first parse blocker), #793 (compat roadmap).