Skip to content

chore: pass --import tsx to node --test via NODE_OPTIONS - #438

Open
PerryLink wants to merge 1 commit into
satorijs:v4from
PerryLink:fix/v4-test-tsx-import
Open

PerryLink wants to merge 1 commit into
satorijs:v4from
PerryLink:fix/v4-test-tsx-import

Conversation

@PerryLink

@PerryLink PerryLink commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Problem

test runs yakumo test --import tsx, but the loader never reaches the node --test child process. yakumo's CLI splices every --import <path> argument out of process.argv and imports it into its own process; the test command then spawns node --test with only the options that are still present. The child therefore gets no TypeScript loader and dies on the first .ts file:

code: 'ERR_UNSUPPORTED_DIR_IMPORT'
url: 'file:///.../packages/element/src'
✖ packages/element/tests/segment.spec.ts

This is exactly the failure visible on the current v4 head in CI: test (20) and test (22) both end with ERR_UNKNOWN_FILE_EXTENSION before any test runs.

Fix

Put the loader in NODE_OPTIONS, which the spawned node --test inherits:

-    "test": "yakumo test --import tsx",
+    "test": "NODE_OPTIONS=\"--import tsx\" yakumo test",

One line. POSIX shell syntax is fine here — CI runs on ubuntu-latest, and the existing scripts already rely on a POSIX shell.

Relation to #433

#433 carries the same change against main. main is no longer where development happens: its install has been broken since 2026-10-07 on an unpublished dependency, and active work moved to v4. This PR is the v4 version of that fix; #433 is superseded by it.

Verification (local, Node 24.22.1)

Before, with the old script:

$ pnpm test element/segment
code: 'ERR_UNSUPPORTED_DIR_IMPORT',
url: 'file:///tmp/wk-satori/satori/packages/element/src'
✖ packages/element/tests/segment.spec.ts
ℹ pass 0
ℹ fail 1

After:

$ pnpm test element/segment
ℹ tests 13
ℹ pass 13
ℹ fail 0

Also:

  • pnpm exec yakumo build element — exit 0 (lib/index.cjs, lib/index.mjs, lib/index.d.ts emitted)
  • pnpm lint — exit 0 (no output)

CI

On v4's head the test job was red for this reason — it died at load time, not on an assertion (run 40496672330). With this PR all relevant checks pass (40895466269):

  • test (20) — success
  • test (22) — success (13 tests, 13 pass, 0 fail; @satorijs/element and @satorijs/protocol both load)
  • lint — success
  • build — success
  • publish — skipped on pull requests

For completeness: the publish job is red on the v4 head run, but that is a pre-existing failure unrelated to this one-line script change. The test job's red state was not an unrelated pre-existing assertion failure — it was this loader bug, and it is fixed here.

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (v4@fa5ec61). Learn more about missing BASE report.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@          Coverage Diff          @@
##             v4     #438   +/-   ##
=====================================
  Coverage      ?   92.53%           
=====================================
  Files         ?        2           
  Lines         ?     1072           
  Branches      ?      132           
=====================================
  Hits          ?      992           
  Misses        ?       80           
  Partials      ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

PerryLink added a commit to PerryLink/satori that referenced this pull request Oct 29, 2026
Same one-line script change as satorijs#438. It is only here so this branch can run its own tests before satorijs#438 lands; drop it when rebasing on v4 after satorijs#438 is merged.
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.

2 participants