Skip to content

fix(fs): wire fs.appendFileSync end-to-end (closes #226) - #231

Merged
proggeramlug merged 1 commit into
PerryTS:mainfrom
TheHypnoo:fix/fs-appendfile-dispatch
Apr 28, 2026
Merged

proggeramlug merged 1 commit into
PerryTS:mainfrom
TheHypnoo:fix/fs-appendfile-dispatch

Conversation

@TheHypnoo

@TheHypnoo TheHypnoo commented Apr 28, 2026 •

Copy link
Copy Markdown
Member

Closes #226.

What was broken

Pre-fix every fs.appendFileSync(path, content) wrote 0 bytes — the file wasn't even created on a fresh path. Confirmed against node --experimental-strip-types (Node correctly accumulates content, Perry produced an empty file).

Why the issue's diagnosis was incomplete

The issue body pointed at crates/perry-codegen/src/expr.rs next to FsWriteFileSync — that arm was indeed missing. But adding it alone changes nothing for the common usage shape, because the HIR Expr::FsAppendFileSync variant is never emitted for namespace imports.

The SWC→HIR lowerer in crates/perry-hir/src/lower/expr_call.rs has two paths for fs.X(...) calls:

  • A named-import path (line ~3884) for import { appendFileSync } from "fs" — already had the appendFileSync arm.
  • A namespace-import path (line ~851) for import * as fs from "fs"; fs.appendFileSync(...) — had arms for writeFileSync / existsSync / mkdirSync / unlinkSync / rmRecursive / readFileBuffer, but not appendFileSync. Without this, fs.appendFileSync(path, content) slipped past as a generic Call { callee: PropertyGet { object: NativeModuleRef("fs"), property: "appendFileSync" }, args: [...] } and the typed variant never got emitted.

That namespace shape is what most user code actually writes (import * as fs from "fs"), so the bug surfaced as "every call writes 0 bytes" in practice. writeFileSync works because it's in both paths; appendFileSync was in only one.

The runtime function (crates/perry-runtime/src/fs.rs::js_fs_append_file_sync) was already correct — OpenOptions::new().create(true).append(true).write_all(content).

What changed

  1. HIR lowerer — added appendFileSync arm to the namespace-import path next to writeFileSync.
  2. LLVM codegen — added Expr::FsAppendFileSync arm next to FsWriteFileSync. Same shape: lower path + content, call js_fs_append_file_sync, discard the i32 status.
  3. runtime_decls — declared js_fs_append_file_sync so the LLVM module can resolve the symbol at link.
  4. Regression test at test-files/test_fs_appendfile.ts exercising the common import * as fs from "fs" shape with three sequential appends.

JS and WASM backends already had arms for the HIR variant since it was originally added, so they're unaffected.

Verified

cargo build --release -p perry clean; cargo test -p perry-hir -p perry-codegen --lib green (20 tests, no regressions).

Byte-for-byte match against node --experimental-strip-types on the regression test:

perry:                  node:
line one                line one
                        <blank>
line one                line one
line two                line two
                        <blank>
line one                line one
line two                line two
line three              line three

Test plan

  • cargo test --workspace --exclude perry-ui-ios --exclude perry-ui-tvos --exclude perry-ui-watchos --exclude perry-ui-gtk4 --exclude perry-ui-android --exclude perry-ui-windows
  • Manual: compile test-files/test_fs_appendfile.ts with the maintainer's local perry and confirm output matches Node
  • At merge: maintainer bumps version + adds CLAUDE.md entry per CONTRIBUTING.md

Closes PerryTS#226.

Pre-fix every `fs.appendFileSync(path, content)` wrote 0 bytes — the
file wasn't even created on a fresh path. The runtime function
(`crates/perry-runtime/src/fs.rs::js_fs_append_file_sync`) was already
correct (`OpenOptions::new().create(true).append(true).write_all(...)`),
the gap was in two earlier passes.

The HIR carries a dedicated `Expr::FsAppendFileSync(path, content)`
variant for this. The named-import path of the SWC→HIR lowerer
already produced it (`import { appendFileSync } from "fs"`), and the
JS / WASM backends both had codegen arms for it. Two things on the
LLVM track were missing:

1. `crates/perry-hir/src/lower/expr_call.rs` — the namespace-import
   path (`import * as fs from "fs"; fs.appendFileSync(...)`) had arms
   for `writeFileSync` / `existsSync` / `mkdirSync` / `unlinkSync`
   etc. but not `appendFileSync`. Without this, the call slipped past
   as a generic `Call { callee: PropertyGet { object:
   NativeModuleRef("fs"), property: "appendFileSync" }, args: [...] }`
   and `Expr::FsAppendFileSync` never got emitted for the most common
   usage shape. Added the arm next to `writeFileSync`.

2. `crates/perry-codegen/src/expr.rs` — even when the HIR variant
   was emitted (named-import path), the LLVM backend had no arm next
   to `Expr::FsWriteFileSync`. The call fell through and was silently
   dropped. Added the arm; same shape as `FsWriteFileSync`, calls
   `js_fs_append_file_sync`.

Plus the runtime declaration in `crates/perry-codegen/src/runtime_decls.rs`
so the LLVM module declares `js_fs_append_file_sync` for the linker.

# Verified

`cargo build --release -p perry` clean, `cargo test -p perry-hir
-p perry-codegen --lib` green (20 tests, no regressions).

End-to-end against `node --experimental-strip-types` on the new
`test-files/test_fs_appendfile.ts`:

    perry:                  node:
    line one                line one
                            <blank>
    line one                line one
    line two                line two
                            <blank>
    line one                line one
    line two                line two
    line three              line three

Identical output.

# Issue diagnosis correction

The issue body diagnosed only the codegen-arm-faltante (path 2 above).
While iterating on this fix I hit the surprising case where adding
just that arm changed nothing for `import * as fs from "fs"; fs.append(...)`.
Tracing the HIR confirmed the namespace-import path needed extending
too. Both arms in this commit; either alone is insufficient for the
common usage pattern.
@proggeramlug

Copy link
Copy Markdown
Contributor

Doc tests fail (expected, okay), once mac and linux greenlight will merge!

@proggeramlug
proggeramlug merged commit 338aa33 into PerryTS:main Apr 28, 2026
7 of 8 checks passed
@TheHypnoo
TheHypnoo deleted the fix/fs-appendfile-dispatch branch April 28, 2026 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fs.appendFileSync writes 0 bytes on native target — HIR variant has no LLVM codegen arm

2 participants