Skip to content

Add mention of CORERUN_IN_BROWSER variable#126143

Merged
MichalStrehovsky merged 1 commit intomainfrom
MichalStrehovsky-patch-1
Mar 26, 2026
Merged

Add mention of CORERUN_IN_BROWSER variable#126143
MichalStrehovsky merged 1 commit intomainfrom
MichalStrehovsky-patch-1

Conversation

@MichalStrehovsky
Copy link
Copy Markdown
Member

The puzzle was missing a piece.

I also moved the paragraph about copying files since it needs to happen before dotnet-serve.

Cc @pavelsavara @radekdoulik

I was scratching my head over how this should work, this was the missing piece.

I also moved the paragraph about copying files since it needs to happen before dotnet-serve.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the CoreCLR WebAssembly build/run documentation to cover the missing CORERUN_IN_BROWSER toggle and to reorder steps so required file copying happens before serving the output directory.

Changes:

  • Document enabling the browser workflow by setting CORERUN_IN_BROWSER in corerun’s CMake configuration.
  • Move the “populate WASM_PRELOAD_DIR / copy IL assets” guidance earlier so it occurs before dotnet-serve.
  • Clarify that corerun.html is the browser entry point and may be missing if the browser workflow wasn’t enabled.

Comment thread docs/workflow/building/coreclr/wasm.md
Comment thread docs/workflow/building/coreclr/wasm.md
@github-actions
Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #126143

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: Justified. The CORERUN_IN_BROWSER CMake variable defaults to 0, meaning the browser workflow won't produce corerun.html unless explicitly enabled — but the previous documentation never mentioned this prerequisite. A developer following the "Browser Testing" section would hit a confusing failure.

Approach: Sound. The change front-loads the CORERUN_IN_BROWSER prerequisite before the dotnet-serve usage instructions, and moves the WASM_PRELOAD_DIR paragraph earlier so the steps follow a logical "configure → populate → serve" order. The added note about corerun.html being the entry point further improves discoverability.

Summary: ✅ LGTM. This is a small, focused documentation improvement. The factual claims are accurate (verified CORERUN_IN_BROWSER defaults to 0 at line 5 of src/coreclr/hosts/corerun/CMakeLists.txt and WASM_PRELOAD_DIR at line 101). No code changes, no API surface changes, no trailing whitespace issues. The reorganization improves the reader experience by placing prerequisites before usage instructions.


Detailed Findings

✅ Accuracy — Variable references verified

Both CORERUN_IN_BROWSER (line 5, defaults to 0) and WASM_PRELOAD_DIR (line 101) exist in src/coreclr/hosts/corerun/CMakeLists.txt exactly as described. The instruction to "set the CORERUN_IN_BROWSER variable to 1" is correct.

✅ Structure — Improved logical flow

Moving prerequisites (CORERUN_IN_BROWSER, WASM_PRELOAD_DIR population) above the dotnet-serve instructions puts them in the correct order for someone following the guide step-by-step. Previously, a reader would start dotnet-serve only to discover afterwards that the necessary files weren't generated.

✅ Formatting — Clean markdown

No trailing whitespace, proper blank-line separation between paragraphs, backtick formatting for variable names and file paths is consistent with the rest of the document.

Generated by Code Review for issue #126143 ·

@MichalStrehovsky MichalStrehovsky merged commit f9060cb into main Mar 26, 2026
29 checks passed
@MichalStrehovsky MichalStrehovsky deleted the MichalStrehovsky-patch-1 branch March 26, 2026 08:33
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 25, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants