React19: Add a validator to check plugin compatibility#552
React19: Add a validator to check plugin compatibility#552leventebalogh merged 15 commits intomainfrom
Conversation
academo
left a comment
There was a problem hiding this comment.
I think most if not all of this check can be a semgrep rule.
|
That's a totally good point, thanks guys, I'll run another circle with this 👍 |
sunker
left a comment
There was a problem hiding this comment.
Just want to flag that Jack's @grafana/react-detect package does exactly this kind of analysis but with a few extras rules. It's published to npm and can be invoked with npx @grafana/react-detect. It also has a --json output mode that could work well for feeding results back into the validator. I think we should use this instead.
Scan plugin module.js bundles for patterns that indicate incompatibility with React 19: removed PropTypes/defaultProps, legacy context, string refs, ReactDOM.render, ReactDOM.findDOMNode, legacy lifecycle methods, and removed React.createFactory.
Replace the in-process regex-based React 19 compatibility checker with a shell-out to npx @grafana/react-detect. This delegates detection logic to the upstream package so rules are maintained in a single place. Key changes: - Dependency changed from modulejs.Analyzer to archive.Analyzer - Runs npx -y @grafana/react-detect@latest --json against the archive - Creates temp dir with dist/ symlink (what react-detect expects) - Dynamic rules from tool output, respecting react19Issue.Disabled config - Graceful skip when npx is not in PATH - 60s timeout, stderr capture for debug logging
The yesoreyeram-infinity-datasource test case was missing the expected provenance recommendation diagnostic, causing the integration test to fail. This was a pre-existing issue on main, not introduced by this PR.
The genreadme test requires the README.md analyzers table to be in sync with registered analyzers. Also revert the incorrect provenance test expectation - the provenance check does not run in Docker CI.
…tput order - Add why-comment on prepareTmpDir explaining archive vs dist/ layout - Add why-comment on react19Issue.Disabled explaining it gates all dynamic rules - Sort source code issue patterns for deterministic diagnostic order - Append false-positive disclaimer to all diagnostic details
9c40676 to
973bf30
Compare
react-detect v0.6.4 added --distDir to point at the built plugin directory directly. This removes the need for creating a temp directory with a dist/ symlink, simplifying the implementation.
Replace the in-process regex-based React 19 compatibility checker with a shell-out to npx @grafana/react-detect. This delegates detection logic to the upstream package so rules are maintained in a single place. Key changes: - Dependency changed from modulejs.Analyzer to archive.Analyzer - Uses --distDir flag (v0.6.4) to point at the archive directly - Dynamic rules from tool output, respecting react19Issue.Disabled config - Stable output order via sorted pattern keys - "React 19 compatibility:" prefix on all diagnostic titles - False-positive disclaimer in all diagnostic details - Graceful skip when npx is not in PATH - 60s timeout, stderr capture for debug logging
- Declare npx as a dependency in ReadmeInfo and README table - Log react-detect stderr at debug level even on success
There was a problem hiding this comment.
Pull request overview
Adds a new reactcompat analyzer to the plugin-validator pipeline to detect potential React 19 compatibility issues in built plugin bundles by invoking @grafana/react-detect and converting its findings into warning diagnostics.
Changes:
- Introduces
pkg/analysis/passes/reactcompatanalyzer that shells out tonpx @grafana/react-detect --jsonand reports findings as diagnostics. - Adds unit tests for JSON parsing and diagnostic reporting behavior.
- Registers the analyzer in the global analyzer list and documents it in the README analyzers table.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| pkg/analysis/passes/reactcompat/reactcompat.go | New analyzer implementation that runs react-detect, parses JSON output, and reports diagnostics. |
| pkg/analysis/passes/reactcompat/reactcompat_test.go | New tests covering parsing and issue-to-diagnostic mapping behavior, plus skip behavior when npx is missing. |
| pkg/analysis/passes/analysis.go | Registers reactcompat in the global analyzers list. |
| README.md | Documents the new analyzer and its dependency on npx. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Use t.Setenv to override PATH instead of skipping when npx is present. This ensures the test runs in all environments.
Dynamic rules now derive their severity from the registered react19Issue rule, so operators can override severity via config.
When npx is not found or react-detect fails, report a warning diagnostic explaining why the check was skipped instead of returning silently.
npx being absent is expected in environments without Node.js (e.g. Docker builder stage). Only emit a diagnostic when npx is found but react-detect fails to execute — that indicates an actual problem.
xnyo
left a comment
There was a problem hiding this comment.
Not tested locally but code lgtm, only a small nit
Makes the pinned version easier to find and bump.
What changed?
Adds a
reactcompatanalyzer that delegates React 19 compatibility detection to@grafana/react-detectvianpx.Why a separate analyzer instead of semgrep rules? We already have a tool for detecting the react-19 compatibility for plugins: @grafana/react-detect, it makes sense to have a single source of truth for the logic.
How it works:
npx -y @grafana/react-detect@latest --json --distDir <archive>against the extracted plugin zipnpxis not available (graceful degradation)How I tested:
Locally with both a react-19 incompatible and compatible plugin bundle, e.g.