Skip to content

[SDK-757] Clear react-router Dependabot alerts in the examples - #173

Open
devtools-agent[bot] wants to merge 2 commits into
mainfrom
sdk-757/clear-react-router-alerts
Open

devtools-agent[bot] wants to merge 2 commits into
mainfrom
sdk-757/clear-react-router-alerts

Conversation

@devtools-agent

@devtools-agent devtools-agent Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Resolves Linear SDK-757: clears all 4 open Dependabot alerts on this repo (Option A from the ticket thread, with the TS example on React 19).

Alerts

All four are for react-router 6.30.6, pulled in by react-router-dom in two examples. None affect the published @rollbar/react package.

Alert Advisory Manifest
#627 GHSA-wrjc-x8rr-h8h6 (open redirect) examples/react-17
#626 GHSA-337j-9hxr-rhxg (SSR hydration) examples/react-17
#633 GHSA-wrjc-x8rr-h8h6 examples/typescript
#719 GHSA-337j-9hxr-rhxg examples/typescript

The first patched release is react-router 7.18.0, and React Router 7 needs React 18+. This is why Dependabot's #167 fails npm ci with ERESOLVE. React Router 8 isn't an option because it needs Node >= 22.22 and CI still covers Node 20.

Changes

examples/react-17: remove React Router

  • No patched React Router supports React 17, and this example exists to show the SDK on React 17. Its routing was just two pages.
  • src/App.jsx now has a small usePathname hook and NavLink built on pushState/popstate. URLs (/, /error-boundary), .active/aria-current styling, the fallback to the playground for unknown paths, and the per-page RollbarContext wrappers all stay as they were.
  • src/index.jsx no longer wraps the app in BrowserRouter.
  • Tests no longer use MemoryRouter. There's a new test for header navigation, and setupTests.js now calls Testing Library's cleanup after each test, because Vitest's afterEach isn't global.

examples/typescript: React 19 + React Router 7

  • react/react-dom ^19.3.0, @types/react/@types/react-dom ^19.3.0.
  • react-router-dom ^6 is replaced by react-router ^7.18.4. In v7, react-router-dom only re-exports react-router.
  • src/index.tsx: ReactDOM.render is replaced by createRoot.
  • @testing-library/react goes from 12 to 16, plus its required @testing-library/dom ^10 peer.
  • Removed ExampleErrors.propTypes, because React 19 ignores propTypes. The prop-types dependency stays because @rollbar/react lists it as a peer.
  • Regenerated both lockfiles with npm 10. The TS lockfile shrinks by about 1,000 lines, mostly from dropping RTL 12's old @testing-library/dom 8 tree.

Validation

Ran locally on Node 20, following the CI steps:

  • npm run install:all -- ci: passes, with no ERESOLVE.
  • npm run lint, lint:examples, typecheck, build:all (including tsc && vite build for the TS example), test, test:examples: all pass. react-17 has 3 tests and typescript has 1.
  • npm audit in both examples: no vulnerabilities. react-17 no longer has any react-router in its lockfile, and typescript resolves react-router@7.18.4.

Review follow-up (8b71633)

  • examples/index.js and the README's "Using with React Router" section used the React Router 5 API (react-router-dom, Switch, <Route> with children). Both now use the React Router 7 API that the TS example uses (react-router, Routes, <Route element>).
    • The snippet's Routes component is renamed to AppRoutes, so it no longer shadows the Routes import.
    • The README was also using [React Router] without ever defining that link, so I added the definition.
  • examples/react-17: trailing slashes. usePathname now strips trailing slashes, as React Router did, so /error-boundary/ shows the ErrorBoundary page and marks its nav link active. There's a new test for this; react-17 now has 4 tests.
  • Re-ran the CI steps on Node 20 with npm 10: install:all -- ci, lint, lint:examples, typecheck, build:all, test and test:examples all pass. Prettier is clean on the changed files.

Follow-ups

🤖 Generated with Claude Code

The four open alerts (GHSA-wrjc-x8rr-h8h6, GHSA-337j-9hxr-rhxg) are for
react-router 6.30.6, pulled in by react-router-dom in two examples. Only
react-router >= 7.18.0 is patched, and React Router 7 requires React 18+.

- examples/react-17: drop react-router-dom. No patched React Router
  supports React 17, and this example exists to show the SDK on React 17.
  App.jsx now switches between its two pages with a small pushState-based
  NavLink, keeping the same URLs, active-link styling and per-page
  RollbarContext wrappers. Adds a navigation test and Testing Library
  cleanup between tests.
- examples/typescript: upgrade to React 19 and react-router 7.18.4
  (imported from `react-router`, which replaces react-router-dom in v7).
  Moves index.tsx to createRoot, bumps @types/react(-dom) to 19 and
  @testing-library/react to 16 (with its @testing-library/dom peer), and
  removes ExampleErrors.propTypes, which React 19 ignores.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Oct 4, 2026

Copy link
Copy Markdown

SDK-757

@rollbar-circleci-machine

Copy link
Copy Markdown
Contributor

AI Agent Review (openai, openai-astra)

Summary

This PR clears the react-router Dependabot alerts in two examples:

  • examples/react-17 no longer depends on react-router-dom. Patched React Router releases need React 18+, so the example now switches between its two pages itself with a small usePathname hook and NavLink component.
  • examples/typescript moves to React 19, react-router 7, @testing-library/react 16 (with @testing-library/dom 10 added as its peer) and @types/react 19. It uses createRoot and drops the propTypes that React 19 ignores.

What I checked

  • react-17 App.jsx: the hand-written usePathname/NavLink behaves like the old routes for this two-page example.
    • Unknown paths still fall back to Playground.
    • Only exact matches get the active class, which still matches .primary-nav a.active (examples/react-17/src/App.css:138).
    • Modified clicks (new tab, new window) are left to the browser, and the browser back button is handled through popstate.
  • react-17 setupTests.js: the explicit afterEach(cleanup) is needed. @testing-library/react 12 only cleans up on its own when afterEach is a global (node_modules/@testing-library/react/dist/index.js:31), and Vitest's isn't by default. Without it, the new navigation test would find duplicate links.
  • React 19 compatibility: scripts/install-all.js:40 links the locally built @rollbar/react into the examples. Its peerDependencies allow 19.x (package.json:109). The library only uses propTypes/defaultProps on class components (src/error-boundary.js:15,24, src/provider.js:27), which React 19 still accepts. ExampleClass still type-checks against index.d.ts:73-77.
  • typescript lockfile: the root entry matches package.json. New transitive packages are present (cookie, set-cookie-parser, dequal). pretty-format still has its nested ansi-styles@5. prop-types correctly stays as a dependency because @rollbar/react needs it as a peer (examples/typescript/package-lock.json:827-831).

Non-blocking notes

  • examples/index.js:10 still imports react-router-dom. It is a standalone snippet outside any example package and is not in this diff, but it now describes an API the examples no longer use.
  • Under React 19 StrictMode, ExampleClass's componentDidMount runs twice in development, so npm start sends 'message from class component' twice. React 17 sent it once. This is expected StrictMode behaviour, not a bug.
  • The react-17 page switch compares paths exactly, so /error-boundary/ (trailing slash) now shows Playground. That doesn't matter for a demo app.

No blocking issues found. I could not run the tests or the build, so this review does not say whether they pass.

@brianr brianr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fix these issues:

examples/index.js:10 still imports react-router-dom. It is a standalone snippet outside any example package and is not in this diff, but it now describes an API the examples no longer use.
The react-17 page switch compares paths exactly, so /error-boundary/ (trailing slash) now shows Playground. That doesn't matter for a demo app.

- examples/index.js and the README's "Using with React Router" section
  still used the React Router 5 API (react-router-dom, Switch, Route
  children). Move both to the React Router 7 API the TypeScript example
  uses (react-router, Routes, Route element), rename the snippet's
  Routes component to AppRoutes so it no longer shadows the import, and
  define the README's missing [React Router] link.
- examples/react-17: ignore trailing slashes in the current path, as
  React Router did, so /error-boundary/ shows the ErrorBoundary page
  instead of falling back to the playground. Adds a test for it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rollbar-circleci-machine

Copy link
Copy Markdown
Contributor

AI Agent Review (openai, openai-astra)

Review: SDK-757, removing react-router from the examples

I read the full diff and the files it touches in the checkout, plus what they depend on: the library source in src/ (Provider, ErrorBoundary, RollbarContext, useRollbarContext), index.d.ts, the CI workflow, scripts/install-all.js and scripts/foreach-example.ts, and each example's vite, eslint and tsconfig files. I found no defects I could confirm.

React 17 example (examples/react-17)

  • react-router-dom, react-router and @remix-run/router are gone from package.json and the lockfile, and nothing in the example still imports them.
  • usePathname, currentPathname and NavLink (examples/react-17/src/App.jsx:109-163) give the same results as the old setup for the links the app has:
    • /error-boundary shows the ErrorBoundary page. The trailing-slash regex keeps / as is and turns /error-boundary/ or // into the right path.
    • Every other path shows the Playground, which matches the old path="*" fallback.
    • Active state and aria-current match the old NavLink end behavior.
    • Clicks with a modifier key or a non-primary button are left to the browser.
    • A popstate listener keeps the page in step with back and forward.
  • Adding afterEach(cleanup) in setupTests.js is correct. The vitest config does not turn on globals, so Testing Library's automatic cleanup never registers. Without it, the new tests would find duplicate Error boundary links.
  • Minor difference: React Router 6 matched paths case-insensitively and this code does not. It doesn't matter for an example app.

TypeScript example (examples/typescript)

  • @rollbar/react 1.0.0 lists react 16.x || 17.x || 18.x || 19.x and prop-types as peer dependencies (examples/typescript/package-lock.json:827-831), so React 19 is supported and keeping prop-types is right.
  • The local library build that CI's yalc link step puts into the example only uses things React 19 still supports: class contextType, class defaultProps, and propTypes, which React 19 simply ignores.
  • Removing ExampleErrors.propTypes and switching to createRoot are the changes React 19 needs.
  • @testing-library/dom was added because @testing-library/react 16 needs it as a peer dependency. The lockfile has every package it depends on.
  • The unchanged TypeScript files have nothing that breaks under @types/react 19, and index.d.ts doesn't use the global JSX namespace.
  • One behavior change: in dev, React 19's StrictMode with createRoot mounts components twice, so ExampleClass sends message from class component twice per page load. That is normal React 18+ dev behavior, not a bug.

Docs and snippets

  • The README and examples/index.js snippets now use the React Router 7 API (Routes, element=, importing from react-router). Renaming Routes to AppRoutes avoids clashing with the imported Routes.
  • The README's [React Router] link had no definition before. It now has one at README.md:694.
  • examples/ is listed in .eslintignore, so examples/index.js isn't linted by the root config.

The diff doesn't show whether tests or CI passed, so I make no claim about that.

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