Skip to content

fix(ext/node): expose internal/tty as requireable module - #34105

Merged
littledivy merged 3 commits into
mainfrom
orch/issue-97
May 16, 2026
Merged

littledivy merged 3 commits into
mainfrom
orch/issue-97

Conversation

@divybot

@divybot divybot commented May 16, 2026

Copy link
Copy Markdown
Contributor

Closes denoland/orchid#97

Some Node.js compatibility tests (e.g. test-output-assertion-color-tty,
test-output-coverage-width-80-color) `require('internal/tty')` to detect
TTY color support before deciding whether to run. This previously failed
with "Cannot find module 'internal/tty'".

Wire `internal/tty` into the CJS require map so it resolves to the
existing ESM polyfill at `ext:deno_node/internal/tty.js`, and expose
`isatty`, `hasColors`, and `getColorDepth` from that module (matching
the surface the tests use). `WriteStream.prototype.hasColors` now
delegates to the standalone `hasColors` function.

Closes denoland/orchid#97
@divybot divybot changed the title node compat: expose internal/tty as requireable module fix(ext/node): expose internal/tty as requireable module May 16, 2026

@fibibot fibibot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. internal/tty now resolves through the native module map, and the exported getColorDepth / hasColors functions match the shape Node uses before node:tty attaches them to WriteStream.prototype; hasColors() still delegates through the same color-depth logic as before.

CI is green.

@fibibot

fibibot commented May 16, 2026

Copy link
Copy Markdown
Contributor

@bartlomieju this is ready to merge

@littledivy

Copy link
Copy Markdown
Member

No tests enabled @divybot?

Enable the three test-runner snapshot tests that previously failed with
"Cannot find module 'internal/tty'":
- test-output-assertion-color-tty.mjs
- test-output-coverage-width-80-color.mjs
- test-output-coverage-width-80-uncovered-lines-color.mjs

All three pass locally now that `require('internal/tty')` resolves.
@divybot

divybot commented May 16, 2026

Copy link
Copy Markdown
Contributor Author

Added the three test-runner snapshot tests that this fix unblocks (test-output-assertion-color-tty, test-output-coverage-width-80-color, test-output-coverage-width-80-uncovered-lines-color) — all pass locally. Pushed as 2653352.

@littledivy

Copy link
Copy Markdown
Member

@divybot resolve conflicts

# Conflicts:
#	tests/node_compat/config.jsonc
@littledivy
littledivy merged commit 9732c5f into main May 16, 2026
136 checks passed
@littledivy
littledivy deleted the orch/issue-97 branch May 16, 2026 21:02
littledivy pushed a commit to crowlKats/deno that referenced this pull request Jun 10, 2026
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.

3 participants