Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PLAT-4872] Swap to a peer dependency on bugsnag/core in plugins #1012

Merged
merged 2 commits into from
Aug 24, 2020

Conversation

imjoehaines
Copy link
Contributor

Goal

Currently our plugins have a hard dependency on @bugsnag/core. This can lead to issues when a plugin is updated separately to @bugsnag/core, because NPM will dutifully install multiple copies of @bugsnag/core — the original version will remain and the plugin will get a newer version of its own. Typically this shouldn't be a problem because changes will be backwards compatible, but it can cause issues for type declarations as a newer @bugsnag/core may be incompatible with an old @bugsnag/core

To prevent having multiple copies of @bugsnag/core, we can use peerDependencies instead of a hard dependency. This should mean that there is only ever one top-level version installed, which will be pulled in by @bugsnag/js, @bugsnag/expo or @bugsnag/react-native

Design

I went with a version constraint of ^7.0.0 to be as compatible as possible, but I think this could be ^7.3.0 (or even the version this is released in (^7.4.0 ?)) because there shouldn't be a scenario where a new plugin version is installed with an old version of core — not 100% sure this is a good idea though!

Changeset

  • All delivery- and plugin- packages no longer have a hard dependency on @bugsnag/core
  • All delivery- and plugin- packages have a dev dependency on @bugsnag/core
  • All delivery- and plugin- packages have a peer dependency on @bugsnag/core

Testing

This should be covered by existing tests

@imjoehaines imjoehaines changed the title Swap to a peer dependency on bugsnag/core in plugins [PLAT-4872] Swap to a peer dependency on bugsnag/core in plugins Aug 20, 2020
@bugsnagbot
Copy link
Collaborator

bugsnagbot commented Aug 20, 2020

@bugsnag/browser bundle size diff

Minified Minfied + Gzipped
Before 39.62 kB 12.23 kB
After 39.62 kB 12.23 kB
± No change No change

Generated by 🚫 dangerJS against 05bad90

@imjoehaines imjoehaines marked this pull request as ready for review August 21, 2020 08:06
@imjoehaines imjoehaines requested a review from a team as a code owner August 21, 2020 08:06
@twometresteve
Copy link
Contributor

Approved from the test perspective, but I have not performed a code review.

Copy link
Contributor

@bengourley bengourley left a comment

Choose a reason for hiding this comment

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

I think going with ^7.0.0 is the right call.

It would be good to keep an eye on whether lerna touches this during a version bump. I don't think it should.

Just needs a changelog entry.

@imjoehaines
Copy link
Contributor Author

It would be good to keep an eye on whether lerna touches this during a version bump. I don't think it should.

Yeah I think it should leave them alone (🤞) — other repos using Lerna have a similar setup

@bengourley bengourley merged commit 57d7a94 into next Aug 24, 2020
@bengourley bengourley deleted the peer-dependencies branch August 24, 2020 08:27
@imjoehaines imjoehaines mentioned this pull request Aug 26, 2020
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.

4 participants