Fix historyContext and RollbarContext typings, type-check index.d.ts in CI (#69)

[devtools-agent]

Fixes #69. Builds on #157 (now merged).

Why

#69 listed several places where index.d.ts disagreed with the implementation:

• The fallbackUI complaint was already fixed in fix ErrorBoundary fallbackUI prop type definition #95.
• [SDK-664] Upgrade rollbar to 3.1.0 to clear Dependabot alerts #157 made the historyContext options and v4action optional.
• Still wrong after [SDK-664] Upgrade rollbar to 3.1.0 to clear Dependabot alerts #157:
  • formatter and filter received location: string, but they're called with a Location object.
  • The returned listener's first argument was typed { action: string; filter: ... }, a copy-paste mistake.
  • There was no history v5 ({ location, action }) signature. [SDK-664] Upgrade rollbar to 3.1.0 to clear Dependabot alerts #157's own test had to cast as never on both calls to get past these types.
• RollbarContext's typings drifted from src/rollbar-context.js: context was optional (the component requires it), and onRender was missing.
• Nothing in CI type-checked index.d.ts, which is how this drift survived for years.

What changed

index.d.ts

• New exported HistoryLocation: pathname, search, hash, optional state and key. It's structural, so history v4 and v5 locations both fit without adding a dependency on history.
• New exported HistoryContextOptions. formatter and filter receive (location: HistoryLocation, action: string). They're declared with method syntax, so callbacks annotated with history's own Location/Action types (v4 or v5) are accepted too.
• New exported HistoryContextListener, an overloaded callable:
  • v4: (location, action)
  • v5: ({ location, action })
  • history.listen(historyContext(rollbar)) now type-checks with both majors.
• RollbarContext: context: string is now required, onRender?: boolean is added, and children is now optional (children: PropTypes.node at runtime).
  • This can break TS consumers who omit context. Omitting it already fails propTypes at runtime and sets the context to undefined.

Type checking

• New scripts/typecheck.ts, run as npm run typecheck and added as a CI step before Build.
  • It type-checks the project with the compiler API and fails on any error outside node_modules.
  • Plain tsc --noEmit fails on errors inside rollbar 3's own index.d.ts and rrweb's @types/css-font-loading-module (the upstream follow-up noted in [SDK-664] Upgrade rollbar to 3.1.0 to clear Dependabot alerts #157).
  • --skipLibCheck would skip our own index.d.ts too. A broken import there would silently become any for consumers, which is exactly what main's import { Callback, Configuration } from 'rollbar' does under rollbar 3.
• New src/tests/types.test-d.tsx: compile-only checks. It isn't collected by Jest.
  • It models the real listen signatures of @types/history@4.7.11 and history@5.3.0.
  • Positive cases, plus @ts-expect-error cases that fail if the typings loosen again. These cover historyContext, fallbackUI and RollbarContext.
• src/tests/history-context.test.ts: dropped the as never casts. The v4 and v5 listener calls now type-check as written.

Validation

Node 20.19, on top of #157's head 1ba8a01:

• npm run typecheck: 10 files, no errors
• Negative checks, to show the step actually catches things:
  • With [SDK-664] Upgrade rollbar to 3.1.0 to clear Dependabot alerts #157's index.d.ts restored, typecheck fails with 11 errors covering every TypeScript types don't match implementation #69 item: the listener shape, location as a string, unused @ts-expect-errors, and RollbarContext.
  • With main's import Rollbar, { Callback, Configuration } from 'rollbar' restored, it fails with TS2614 inside index.d.ts. tsc --noEmit --skipLibCheck reports that same file as clean.
• npm run lint -- --max-warnings 0: pass
• Prettier: all changed files pass
• npx jest: 13/13 passing (3 suites)
• npm run build: pass
• examples/typescript against the yalc-published build:
  • tsc && vite build: pass
  • vitest run: 1/1
  • eslint: pass

Only examples/typescript was built locally. The other examples don't use historyContext or the TS types of RollbarContext.

Review follow-up

• scripts/typecheck.ts now splits SourceFile.fileName on / instead of path.sep. TypeScript stores file names with forward slashes on every platform, so on Windows the old split found no node_modules segment and dependency errors were reported. Checked with the compiler API: a root name of C:\repo\node_modules\x\index.d.ts is stored as C:/repo/node_modules/x/index.d.ts.
• RollbarContext children is now optional. There's a new compile-time case for <RollbarContext context="/page" />: with children required again, typecheck fails with TS2769.
• Re-run on Node 20.19: typecheck, lint --max-warnings 0, Prettier, jest (13/13) and build all pass.

Review follow-up 2

• formatter and filter now use method syntax (Brian's c4ee130). Under strictFunctionTypes, the old function-typed properties rejected callbacks annotated as (location: Location, action: Action) with history's types. Those annotations are correct because the runtime passes history's location and action through unchanged.
• f40530b re-indents formatter? (Prettier failed on it) and adds compile-time cases to types.test-d.tsx for formatter/filter callbacks annotated with the history v4 and v5 Location/Action types.
  • Negative check: with property syntax restored, typecheck fails with TS2322 on all four annotated callbacks.
  • The existing @ts-expect-error cases (a non-string formatter return, filter(location: string)) still error under method syntax.
• Re-run on Node 20.19: typecheck (10 files, no errors), lint --max-warnings 0, Prettier, jest (13/13) and build all pass.

Changelog note: TS users who omit RollbarContext's context, or type formatter/filter with a string argument, will get new compile errors. Both cases were already broken at runtime.

🤖 Generated with Claude Code

[rollbar-circleci-machine]

AI Agent Review (openai, openai-astra)

No findings: the new typings match what the code actually does, and the typecheck setup is correctly wired.

Typings vs. runtime

• historyContext passes a location object to formatter/filter, not a string, and handles both call shapes: (location, action) for history v4 and ({ location, action }) for v5 (src/history-context.js:23-32). The new HistoryLocation / HistoryContextOptions / HistoryContextListener types (index.d.ts:89-112) describe exactly that. The old typing said formatter: (location: string, …), which was wrong.
• The two-signature HistoryContextListener fits a single-signature listen callback from either history version, because TypeScript tries each signature in turn. src/tests/types.test-d.tsx:52-61 checks both versions at compile time.
• RollbarContext now types context as required and adds optional onRender?: boolean. This matches the component's propTypes: context: PropTypes.string.isRequired, onRender: PropTypes.bool (src/rollbar-context.js:8-12).
• The README examples (README.md:506-512) already use location.pathname, so they match the new types. The only other users of historyContext and RollbarContext are in examples/index.js, which is plain JS and isn't affected.

Typecheck wiring

• tsconfig.json has no include, so the program covers every .ts/.tsx/.d.ts except node_modules and examples. That brings in index.d.ts, scripts/typecheck.ts and src/tests/types.test-d.tsx.
• ./rollbar-react resolves to the root index.d.ts (src/tests/rollbar-react/package.json:3).
• Jest's default test file pattern needs a name ending in test.tsx or spec.tsx, so types.test-d.tsx is not run by Jest (jest.config.js:5).
• The comment in the script is right that --skipLibCheck would also skip checking our own index.d.ts, so filtering errors by file path is the right approach.
• @types/react@17.0.76 (package-lock.json:4208-4209) and @types/node are in the lockfile, which the JSX test file and the script's use of __dirname both need.

Non-blocking notes (not findings)

• scripts/typecheck.ts:30 detects node_modules by splitting the path on path.sep. TypeScript normally stores file names with forward slashes on every platform, so on Windows this probably wouldn't filter out errors inside dependencies. CI runs only on ubuntu-latest, so CI isn't affected. A regex like /[\\/]node_modules[\\/]/ would work everywhere. I couldn't confirm this against the TypeScript source because node_modules isn't installed in this checkout.
• Making context required is a breaking change at the type level for TypeScript users who left it out. The runtime already required it, so this is a correction, but it may be worth a changelog line.
• Existing issue on a line this PR doesn't change: RollbarContext types children: ReactNode as required (index.d.ts:44), while the propTypes treat it as optional (src/rollbar-context.js:11).

The diff doesn't show whether npm run typecheck, npm test or lint pass, and I couldn't run them here.

[rollbar-circleci-machine]

AI Agent Review (openai, openai-astra)

Review of #162: fix the historyContext / RollbarContext typings and type-check index.d.ts in CI

The typings now match the runtime.

• historyContext: src/history-context.js:23-37 passes a location object to formatter/filter and accepts both the history v4 (location, action) and v5 ({ location, action }) listener forms. The new HistoryLocation / HistoryContextOptions / HistoryContextListener types in index.d.ts:89-112 describe exactly that. Under strict, a v4 listen matches the first overload and a v5 listen matches the second.
• RollbarContext: context is now required and onRender is added, matching the propTypes in src/rollbar-context.js:8-12.
• Compatibility note: TypeScript users who left out context, or typed formatter/filter with a string argument, will get new compile errors. That is the intended fix, since both already failed at runtime, but it may be worth a changelog line.

The compile-time tests look sound.

• src/tests/types.test-d.tsx doesn't match Jest's default test pattern (roots at jest.config.js:5) or the testing-library ESLint override (.eslintrc.json:36-37), so only npm run typecheck runs it.
• It imports the real index.d.ts through src/tests/rollbar-react/package.json:3.
• Each @ts-expect-error sits directly above the line where the error is reported, so the directive will be flagged as unused if the typings get looser.
• The updated history-context.test.ts calls type-check without the old as never casts.

scripts/typecheck.ts works on Linux CI but not on Windows. The approach is reasonable: a full program check, then drop diagnostics from files under node_modules, instead of skipLibCheck, which would also skip index.d.ts. The API calls look right for the pinned typescript 5.3.3 (package-lock.json:13003-13004). The one problem: the filter splits file names on path.sep. TypeScript always stores file names with /, so on Windows nothing is filtered and the script fails on the third-party typing errors it is meant to ignore. CI runs on ubuntu-latest, so CI is not affected.

Outside the changed lines (not a finding): index.d.ts:44 still requires children: ReactNode on RollbarContext, while the propTypes at src/rollbar-context.js:11 treat it as optional. This was already the case before this PR.

I didn't run anything for this review, and the diff doesn't show whether npm run typecheck, lint or the tests pass.

[rollbar-circleci-machine]

AI Agent Review (openai, openai-astra)

No confirmed defects. This looks good to merge.

The new index.d.ts typings now match what the code does at runtime:

• historyContext: src/history-context.js:23-37 calls formatter/filter with the history location object (the old typings said string). It returns one listener that handles both v4 (location, action) and v5 ({ location, action }) calls. HistoryContextListener models this with two overloads. Under strict (tsconfig.json:6), a v4-style listen matches the first overload. A v5-style listen matches the second, because the first needs two arguments.
• RollbarContext: context is now required and onRender?: boolean is added. This matches the propTypes (context: PropTypes.string.isRequired, onRender: PropTypes.bool) in src/rollbar-context.js:8-12. It is a breaking change at the type level for TS users who left out context, but runtime propTypes already warned about that case.
• history-context.test.ts: the as never casts are gone, and the test calls now compile against the real listener type.
• types.test-d.tsx: tsconfig.json has no include, so this file is type-checked by the new script. Jest's default testMatch does not match it, so it never runs as a test. It reaches the root index.d.ts through src/tests/rollbar-react/package.json:3. Each @ts-expect-error is on the line just above the line where its error would be reported.
• scripts/typecheck.ts and CI: the script uses the public TypeScript API (typescript 5.3.3 is pinned at package-lock.json:13004). It type-checks the project, including our own index.d.ts, and ignores only diagnostics located under node_modules. The CI step runs before the build on every node version in the matrix.

This diff doesn't show CI results, so I can't confirm the new type check passes.

Non-blocking notes (I couldn't read the TypeScript source here because node_modules isn't checked out):

• scripts/typecheck.ts:30 splits fileName on path.sep. As far as I know, TypeScript always uses / in file names, even on Windows. If so, the node_modules filter would do nothing for Windows contributors running this locally. CI uses ubuntu only, so CI is not affected. Checking for /node_modules/ (or splitting on /[\\/]/) would work on every OS.
• HistoryLocation makes state and key optional, and formatter/filter take action: string. That is what lets listen from both history versions accept the listener. The flip side: a user who annotates their formatter parameters with history's own Location or Action types would probably get a type error. Unannotated callbacks (the usage shown in the README) infer the right types. Worth keeping in mind if someone reports it.

[rollbar-circleci-machine]

AI Agent Review (openai, openai-astra)

Review: typings fix for historyContext / RollbarContext, plus a CI type check

I read index.d.ts, the runtime code (src/history-context.js, src/rollbar-context.js), tsconfig.json, jest.config.js, the ESLint config, the CI workflow and the new/changed tests.

What checks out:

• RollbarContext props now match the runtime propTypes: context is required, onRender is an optional bool, children is optional (src/rollbar-context.js:8-12). Making context required breaks TS consumers who left it out, but it matches the runtime contract. It may be worth a changelog note.
• The two call forms on HistoryContextListener match what the runtime does (src/history-context.js:23-28). Under TS's rules for assigning overloaded functions, the listener can be passed to the v4 (location, action) shape and the v5 ({ location, action }) shape used in types.test-d.tsx.
• Jest won't pick up types.test-d.tsx: it doesn't match the default testMatch, and jest.config.js doesn't override it. Each @ts-expect-error sits on the line where TS reports the error: the JSX opening tag or attribute, or the single-line call.
• scripts/typecheck.ts reads tsconfig.json, which has no include, so root index.d.ts, scripts/ and src/tests/ are all checked, and examples and node_modules are excluded. The TS API calls look right for the pinned TypeScript 5.3.3 (package-lock.json:13004). The build outputs (dist, lib, bundles) are JS only, so they don't leak into the program.

Could not verify: node_modules is not installed in this checkout, so I couldn't run npm run typecheck, Jest or ESLint. The diff doesn't show any results either.

One low-severity finding: formatter and filter are declared as function-typed properties, so with strict on TS checks their parameters contravariantly. The README tells users that location/action are history.location/history.action. If they annotate their callbacks with the history package's Location/Action types, compilation fails. Declaring the two options with method syntax fixes it and keeps the new @ts-expect-error cases failing as intended.

[aborek-rollbar]

[P3] Accept callbacks annotated with history’s own types

formatter and filter are function-typed properties, so under strictFunctionTypes, callbacks annotated as (location: Location, action: Action) are rejected because they are narrower than HistoryLocation and string. These annotations are valid because the runtime passes the actual history location and action through unchanged. Using method syntax—formatter?(...): string and filter?(...): boolean—would preserve type safety while accepting callbacks typed with either history v4 or v5 types. A compile-time test covering these annotated callbacks would prevent regressions.

[devtools-agent]

Thanks, this is addressed:

• c4ee130 switches formatter and filter in HistoryContextOptions to method syntax, so callbacks typed with history's own Location/Action (v4 or v5) are now accepted.
• f40530b adds compile-time cases to src/tests/types.test-d.tsx for formatter/filter callbacks annotated with the history v4 and v5 Location/Action types. With the old property syntax restored, npm run typecheck fails with TS2322 on all four. The existing @ts-expect-error cases (a non-string formatter return, filter(location: string)) still error.

typecheck, lint --max-warnings 0, Prettier, jest (13/13) and build pass on Node 20.19.

[rollbar-circleci-machine]

AI Agent Review (openai, openai-astra)

No defects found; findings are empty. I did not run tsc, Jest or ESLint, so this review is based only on reading the code.

historyContext typings match the runtime. src/history-context.js:23-32 passes the location object to filter/formatter and handles both history v4 (location, action) and v5 ({ location, action }). The new HistoryLocation / HistoryContextOptions / HistoryContextListener in index.d.ts:89-112 describe exactly that. The v4 overload has two required parameters, so v5's one-argument listen resolves to the second overload. formatter/filter are declared as methods, which lets callbacks typed with history's own Location/Action compile, while a (location: string) callback still fails.

RollbarContext typings match its propTypes. src/rollbar-context.js:8-12 marks context as required and onRender/children as optional, which is what index.d.ts:43-47 now declares.

The type tests and typecheck script do what they claim. Each @ts-expect-error in src/tests/types.test-d.tsx sits on the line just before where the compiler reports the error, so a loosened typing would show up as an unused directive. Jest will not pick up *.test-d.tsx: jest.config.js keeps the default test pattern. tsconfig.json has no include, so npm run typecheck checks everything outside node_modules and examples, including index.d.ts. scripts/typecheck.ts drops errors from files under node_modules but keeps errors that are not tied to a file.

Notes (not defects):

• Users who type the formatter/filter location as a string, or leave out context on RollbarContext, will now get compile errors. The runtime already behaved that way, but it is worth a changelog/release note.
• @types/react, which index.d.ts and the new type test need, is not a direct devDependency; it comes in through @types/react-dom (package-lock.json:4233-4239). The existing ts-jest tests already depended on it.

[rollbar-circleci-machine]

AI Agent Review (openai, openai-astra)

No problems worth an inline comment. index.d.ts now matches what the code actually does at runtime.

• historyContext: the listener's two call forms are the v4 (location, action) and v5 ({ location, action }) branches in src/history-context.js:23-28. formatter/filter receive the location object, not a string (src/history-context.js:29-32), which matches README.md:481.
• RollbarContext: the props now match the propTypes: context required, onRender/children optional (src/rollbar-context.js:8-12).
• Method syntax is deliberate: HistoryContextOptions declares formatter/filter as methods, so their parameters are checked both ways. That is what lets callbacks typed with history's own Location/Action compile. types.test-d.tsx:61-72 covers this and would fail if someone switched to arrow-property syntax.
• Test file: each @ts-expect-error in types.test-d.tsx should be a real type error with TS 5.3.3 and @types/react 17.0.76, so an unused one would fail the check. Jest won't pick up types.test-d.tsx as a test (jest.config.js:5). It follows the ESLint rules in .eslintrc.json:41-66.
• Typecheck script: tsconfig.json has no include, so the check covers index.d.ts, scripts/ and src/tests/. Imports resolve through src/tests/rollbar-react/package.json:3, so no build step is needed before it runs.

Non-blocking notes

1. Making context required will break TS builds for anyone who left it out of <RollbarContext>. That matches the propTypes and README.md:367, but it belongs in the release notes.
2. scripts/typecheck.ts:116-118 drops diagnostics whenever the absolute path contains a node_modules folder. If the repo were ever checked out inside one, every error would be hidden and the check would pass. Filtering on the path relative to the repo root would fix that, if you want to harden it.

I didn't run anything, so I can't say whether CI passes.

[rollbar-circleci-machine]

AI Agent Review LGTM (openai, openai-astra)

LGTM. No blocking findings were found.

LGTM — no findings. I read the diff and the code it touches in the checkout: index.d.ts, src/history-context.js, src/rollbar-context.js, tsconfig.json, jest.config.js, .eslintrc.json, src/tests/rollbar-react/package.json, the CI workflow and the new script and tests. I did not run tsc, Jest or ESLint, so this review does not claim the new CI step passes.

Typings now match the runtime

• RollbarContext props (index.d.ts:43-47): context is required, and onRender and children are optional. This matches the component's propTypes (src/rollbar-context.js:8-12). Making children explicit also keeps working with React 18+ types.
• historyContext (index.d.ts:89-112): the runtime passes history's location object and action to formatter and filter unchanged (src/history-context.js:23-32). The old typings said location was a string, which was wrong.
  • The overloads on HistoryContextListener fit both call shapes: v4 (location, action) and v5 ({ location, action }). The runtime tells them apart at src/history-context.js:24-28.
  • HistoryLocation is structural, so v4 locations (state: unknown, key?) and v5 locations (key required) both fit.
  • The options use method syntax on purpose. That makes their parameters bivariant, so callbacks typed with history's own Location and Action types are accepted. A callback typed (location: string) is still an error, which src/tests/types.test-d.tsx:78 asserts.
• Previous return type: it required a filter property on the listener's argument, so it could not be passed to v5's history.listen without a cast. The new type is strictly more usable.

scripts/typecheck.ts

• tsconfig.json has no include, so the program covers every .ts/.tsx/.d.ts outside node_modules and examples. That is index.d.ts, scripts/*.ts and src/tests/**. ESLint's parserOptions.project also sees the new files.
• The script drops diagnostics only when the diagnostic's file path contains a node_modules segment. Errors with no file (config or global errors) are kept.
• The header comment is right that skipLibCheck would also skip the root index.d.ts.
• It adds errors from the parsed config explicitly. That is needed because the program is not created with configFileParsingDiagnostics.

src/tests/types.test-d.tsx

• Each @ts-expect-error sits on the line directly above where TypeScript reports the error: the call expressions, the fallbackUI attribute, and the RollbarContext tag missing context.
• The .test-d.tsx name does not match Jest's default testMatch, so Jest won't try to run it.
• The unused location parameters come before a used action, so the configured @typescript-eslint/no-unused-vars (default args: after-used) does not flag them.

Outside the diff (not findings)

• The typecheck relies on @types/react, which is not a direct devDependency. The installed 17.0.76 arrives through @testing-library/react → @types/react-dom (see package-lock.json). Existing ts-jest tests already depend on this.
• examples/index.js:149 renders <RollbarContext name=""> without context. It is plain JS, so the stricter typing doesn't affect it.

Consumer impact
Making context required and changing the formatter/filter parameter types are breaking changes for TypeScript consumers who followed the old, incorrect typings. Both changes match what the runtime already required, but they may be worth calling out in release notes.