Skip to content

fix!: remove @libp2p/components #360

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

Conversation

achingbrain
Copy link
Collaborator

@achingbrain achingbrain commented Oct 10, 2022

@libp2p/components is a choke-point for our dependency graph as it depends on every interface, meaning when one interface revs a major
@libp2p/components major has to change too which means every module depending on it also needs a major.

Switch instead to constructor injection of simple objects that let modules declare their dependencies on interfaces directly instead of
indirectly via @libp2p/components

Refs libp2p/js-libp2p-components#6

BREAKING CHANGE: modules no longer implement Initializable instead switching to constructor injection

Updates all `@libp2p/*` deps to the latest versions and configures
dependabot - for some reason it wasn't turned on for dev deps.

BREAKING CHANGE: updates @libp2p/components to the latest major
@achingbrain achingbrain requested a review from a team as a code owner October 10, 2022 11:05
@achingbrain
Copy link
Collaborator Author

The latest @libp2p/components is breaking because the transport and upgrader interfaces have changed. This obviously has nothing to do with pubsub so is not a great state of affairs - there's some discussion here about how we can improve this and allow ourselves to make breaking changes to interfaces that then mean we don't need to upgrade the entire ecosystem.

@achingbrain
Copy link
Collaborator Author

@wemeetagain looks like the e2e test timed out in CI?

`@libp2p/components` is a choke-point for our dependency graph as it depends on every interface, meaning when one interface revs a major
`@libp2p/components` major has to change too which means every module depending on it also needs a major.

Switch instead to constructor injection of simple objects that let modules declare their dependencies on interfaces directly instead of
indirectly via `@libp2p/components`

Refs libp2p/js-libp2p-components#6

BREAKING CHANGE: modules no longer implement `Initializable` instead switching to constructor injection
@achingbrain achingbrain changed the title chore!: update deps and configure dependabot fix!: remove @libp2p/components Oct 12, 2022
@@ -361,7 +375,7 @@ export class GossipSub extends EventEmitter<GossipsubEvents> implements Initiali
cancel: () => void
} | null = null

constructor(options: Partial<GossipsubOpts> = {}) {
constructor(components: GossipSubComponents, options: Partial<GossipsubOpts> = {}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a breaking change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, as noted in the PR title & description

dapplion
dapplion previously approved these changes Oct 14, 2022
@wemeetagain wemeetagain merged commit ac0ad41 into ChainSafe:master Oct 20, 2022
@wemeetagain wemeetagain mentioned this pull request Oct 21, 2022
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.

3 participants