Skip to content

Conversation

@connorshea
Copy link
Contributor

@connorshea connorshea commented Dec 14, 2025

React 19 disallows javascript: URLs entirely, so I have updated the docs accordingly. See https://react.dev/blog/2024/04/25/react-19-upgrade-guide#other-breaking-changes

Also, remove dangerouslySetInnerHTML from the help message. It doesn't seem worth risking people using it by suggesting that.

Copilot AI review requested due to automatic review settings December 14, 2025 03:18
@connorshea connorshea requested a review from camc314 as a code owner December 14, 2025 03:18
@github-actions github-actions bot added A-linter Area - Linter C-docs Category - Documentation. Related to user-facing or internal documentation labels Dec 14, 2025
Copy link
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

This PR updates the react/jsx-no-script-url linter rule to reflect that React 19 now blocks javascript: URLs entirely, rather than warning about a "future version" doing so. The diagnostic messages are made more discouraging of dangerouslySetInnerHTML usage, and the documentation is updated with a reference to the React 19 upgrade guide.

  • Updates diagnostic warning message to state React 19 blocks javascript: URLs (not "future version")
  • Makes help message more discouraging by changing "If you need to" to "If you absolutely need to" and "try using" to "consider"
  • Updates documentation to reference React 19 specifically with link to upgrade guide

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
crates/oxc_linter/src/rules/react/jsx_no_script_url.rs Updates diagnostic messages to reference React 19 specifically, improves documentation with historical context and external link, cleans up test JSON formatting
crates/oxc_linter/src/snapshots/react_jsx_no_script_url.snap Updates snapshot to reflect new diagnostic and help messages

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

@charliecreates charliecreates bot left a comment

Choose a reason for hiding this comment

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

The main concern is the security guidance in the diagnostic help: it may still be read as recommending dangerouslySetInnerHTML as an alternative, which is risky for an XSS-related rule. Tightening the wording to more explicitly discourage it (and mention sanitization/trusted input if referenced) would better align the rule’s intent. Everything else in the diff is straightforward doc/snapshot alignment.

Summary of changes

Summary

This diff updates the react/jsx-no-script-url rule messaging and documentation to align with React 19 behavior:

  • Diagnostic text updated from “a future version of React…” to “React 19 blocks javascript: URLs…”.
  • Help text clarified to recommend event handlers and to more strongly caution around using dangerouslySetInnerHTML.
  • Rule docs refreshed in declare_oxc_lint!:
    • Reformatted/expanded rationale.
    • Added a link to the React 19 upgrade guide stating javascript: URLs are disallowed.
  • Tests and snapshots updated to reflect new messaging and minor JSON formatting changes in test cases.

Files touched:

  • crates/oxc_linter/src/rules/react/jsx_no_script_url.rs
  • crates/oxc_linter/src/snapshots/react_jsx_no_script_url.snap

@charliecreates charliecreates bot removed the request for review from CharlieHelps December 14, 2025 03:23
@codspeed-hq
Copy link

codspeed-hq bot commented Dec 14, 2025

CodSpeed Performance Report

Merging #16817 will not alter performance

Comparing docs-jsx-script (a117535) with main (bbafba5)

Summary

✅ 4 untouched
⏩ 41 skipped1

Footnotes

  1. 41 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@camc314 camc314 self-assigned this Dec 14, 2025
@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label Dec 14, 2025
Copy link
Contributor

camc314 commented Dec 14, 2025

Merge activity

React 19 disallows `javascript:` URLs entirely, so I have updated the docs accordingly. See https://react.dev/blog/2024/04/25/react-19-upgrade-guide#other-breaking-changes

Also, remove `dangerouslySetInnerHTML` from the help message. It doesn't seem worth risking people using it by suggesting that.
@graphite-app graphite-app bot merged commit e8369ef into main Dec 14, 2025
20 checks passed
@graphite-app graphite-app bot deleted the docs-jsx-script branch December 14, 2025 13:47
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Dec 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter Area - Linter C-docs Category - Documentation. Related to user-facing or internal documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants