Skip to content

Conversation

@alii
Copy link
Member

@alii alii commented Jan 24, 2026

Changed ArgumentRequirement from Yes to No in logWithLevel so that console.log(), console.debug(), console.info(), console.warn(), and console.error() with no arguments will still call messageWithTypeAndLevel.

This allows embedders like Bun to handle the empty argument case and print a newline, matching Node.js and browser behavior.

Changed ArgumentRequirement from Yes to No in logWithLevel so that
console.log(), console.debug(), console.info(), console.warn(), and
console.error() with no arguments will still call messageWithTypeAndLevel.

This allows embedders like Bun to handle the empty argument case and
print a newline, matching Node.js and browser behavior.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@alii
Copy link
Member Author

alii commented Jan 24, 2026

/build-preview

@github-actions
Copy link

🔄 Preview build started

Release tag: autobuild-preview-pr-146-dd473861
Workflow: View progress

@Jarred-Sumner Jarred-Sumner merged commit f781085 into oven-sh:main Jan 24, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 24, 2026

Caution

Review failed

The pull request is closed.

Walkthrough

Modified ConsoleClient.cpp's logWithLevel function to change the ArgumentRequirement parameter from Yes to No when calling internalMessageWithTypeAndLevel. This removes the guard preventing logs with zero arguments, allowing console messages without arguments to generate appropriate prefixes.

Changes

Cohort / File(s) Summary
ConsoleClient argument requirement change
Source/JavaScriptCore/runtime/ConsoleClient.cpp
Changed ArgumentRequirement::Yes to ArgumentRequirement::No in logWithLevel's call to internalMessageWithTypeAndLevel, enabling console logs with zero arguments

Possibly related PRs

Suggested reviewers

  • Jarred-Sumner
  • sosukesuzuki

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

⚠️ Preview build was cancelled

This may have been caused by a newer build being triggered for the same PR.

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