Skip to content

[SDK-776] Move prop-types from peerDependencies to dependencies - #178

Open
devtools-agent[bot] wants to merge 1 commit into
mainfrom
sdk-776/prop-types-dependency
Open

devtools-agent[bot] wants to merge 1 commit into
mainfrom
sdk-776/prop-types-dependency

Conversation

@devtools-agent

Copy link
Copy Markdown

Fixes #70. Linear: SDK-776.

What changed

  • package.json: moved prop-types from peerDependencies to dependencies (same ^15.7.2 range). Removed the duplicate devDependencies entry too. Without that, npm keeps prop-types and its dependencies marked "dev": true in the lockfile.
  • rollup.config.mjs: added external: ['prop-types'] to both build configs.
  • package-lock.json: regenerated for the above.

Why

prop-types is used only for our components' runtime prop checks. Consumers don't need to share an instance with us, and the prop-types README recommends that libraries list it in dependencies. Our README's install line is npm install @rollbar/react rollbar, which never mentions prop-types. So anyone whose package manager doesn't auto-install peers gets an import-time crash.

The rollup change is needed because rollup-plugin-peer-deps-external only externalizes peer deps. Moving the package alone made rollup try to inline prop-types' CommonJS source: the ES/CJS build failed outright, and the UMD bundle silently changed. I externalized only prop-types instead of turning on includeDependencies, because that option would also stop bundling tiny-invariant, which is an unrelated behaviour change.

Validation

  • npm run build output (dist/, lib/, bundles/) is byte-identical to the v1.2.0 build (diff -r).
  • npm test (13/13), npm run lint and npm run typecheck pass.
  • Consumer smoke test: a clean project installs only @rollbar/react react@18 react-dom@18 rollbar@3, then SSR-renders <Provider><ErrorBoundary>…:
Installer v1.2.0 (npm registry) this branch (npm pack)
yarn 1 unmet peer dependency "prop-types" → Cannot find module 'prop-types' renders
pnpm (auto-install-peers=false) Cannot find module 'prop-types' renders
npm --legacy-peer-deps Cannot find module 'prop-types' renders

Notes:

  • npx prettier --check rollup.config.mjs already fails on main because of the existing entryFileNames lines. I didn't reformat unrelated code.
  • Users who already list prop-types themselves are unaffected. Within the ^15 range it dedupes to a single copy.

🤖 Generated with Claude Code

prop-types is an implementation detail of the components' runtime prop
checks, not something consumers share with us, so per the prop-types
README it belongs in `dependencies`. As a peer it isn't installed by
yarn 1, pnpm without auto-install-peers, or npm --legacy-peer-deps, and
the README's install instructions don't mention it, so following them
gives "Cannot find module 'prop-types'" at import time.

Keep it external in both rollup builds: peerDepsExternal only covers
peers, and without this rollup would try to inline prop-types' CommonJS
source (breaking the ES/CJS build) and bake it into the UMD bundle.
Build output is byte-identical to 1.2.0.

Also drop the now-redundant devDependency entry, which otherwise keeps
prop-types and its dependencies flagged as dev-only in the lockfile.

Fixes #70

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

linear-code Bot commented Oct 11, 2026

Copy link
Copy Markdown

SDK-776

@rollbar-circleci-machine

Copy link
Copy Markdown
Contributor

AI Agent Review LGTM (openai, openai-astra)

LGTM. No blocking findings were found.

LGTM, no findings.

package.json (package.json:56-59, package.json:107-110): prop-types ^15.7.2 moves from peerDependencies to dependencies, and the duplicate devDependencies entry is removed. The lockfile still resolves to prop-types@15.8.1 (package-lock.json:11734-11743), so dev installs don't change.

rollup.config.mjs: the new external: EXTERNAL (rollup.config.mjs:20, :60, :98) works alongside peerDepsExternal():

  • Its options hook combines your existing external with the peer-deps matcher instead of replacing it (node_modules/rollup-plugin-peer-deps-external/src/index.js:12-22).
  • An array value becomes an exact-id match (node_modules/rollup-plugin-peer-deps-external/src/external-to-fn.js:27-31).
  • All four imports in src/ use the bare 'prop-types' specifier with no subpaths (src/provider.js:4, src/error-boundary.js:4, src/rollbar-context.js:4, src/rollbar-configuration.js:9), so the exact match catches every one.
  • The UMD config's existing 'prop-types': 'PropTypes' global (rollup.config.mjs:56) still applies, and tiny-invariant stays bundled, as the PR says.

package-lock.json: the root entry mirrors package.json. "dev": true is dropped only from prop-types (package-lock.json:11734), its runtime dependencies loose-envify (:10807), object-assign (:11163) and the nested react-is@16.13.1 (:11744), and js-tokens (:10574), which loose-envify needs. The top-level react-is@17.0.2 correctly stays dev-only (:11849-11854).

Not verified: the PR's claims that the build output is byte-identical and that tests, lint and typecheck pass. No CI or test output was available to check them.

Outside the diff, no action needed: examples/typescript/package.json:8 still lists prop-types itself. That stays harmless within ^15.

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.

It is recommended to put prop-types in dependencies

2 participants