Rename AbortController CMake option without breaking existing consumers - #261
Merged
bghgary merged 4 commits intoSep 29, 2026
Merged
Conversation
Preserve the old spelling as a default for the new option and reject conflicting settings to avoid silently enabling a disabled polyfill. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The alias behavior, conflict detection, target gating, and documentation are consistent with the compatibility goal.
Review effort: Balanced
Findings: None
What changed in this PR
Renames the AbortController CMake option while preserving compatibility with existing consumers.
Changes:
- Adds the new option name with legacy-value fallback and conflict detection.
- Updates target gating and documentation.
| File | Description |
|---|---|
CMakeLists.txt |
Defines compatibility and conflict handling. |
Polyfills/CMakeLists.txt |
Uses the renamed option. |
Polyfills/AbortController/Readme.md |
Documents both accepted spellings. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Translate the legacy setting only when the new option is absent. Explicit settings of the new option win without a conflict error. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Show only the new option in the option list and map the old variable onto it afterward. Drop the extra README migration text. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep the new CMake option in the option list and apply the old spelling only to the effective value. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
bkaradzic-microsoft
approved these changes
Sep 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[Created by Copilot on behalf of @bghgary]
Context
The AbortController polyfill option’s spelling is inconsistent with the other polyfill options.
Compatibility
Use
JSRUNTIMEHOST_POLYFILL_ABORTCONTROLLERgoing forward. The old spelling remains supported so existing consumers continue to select the same target; if both names are set, the old value takes precedence.