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

fix: specify pbjs root #323

Merged
merged 4 commits into from
Apr 20, 2021
Merged

fix: specify pbjs root #323

merged 4 commits into from
Apr 20, 2021

Conversation

achingbrain
Copy link
Member

If we do not specify a root for pbjs, the default is used which is
shared globally, so we can't have a protobuf message called Message
that exists for both ipfs-bitswap and libp2p-kad-dht, for example.

Specify a root to add a scope to the declaration of protobuf messages.

If we do not specify a root for pbjs, the default is used which is
shared globally, so we can't have a protobuf message called `Message`
that exists for both `ipfs-bitswap` and `libp2p-kad-dht`, for example.

Specify a root to add a scope to the declaration of protobuf messages.
package.json Outdated Show resolved Hide resolved
Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

This looks good. CI fails seem unrelated?

Should we add root names to every message to be safe?

@achingbrain
Copy link
Member Author

Should we add root names to every message to be safe?

Yes, every use of pbjs should have a root added as this will bite us again..

@achingbrain achingbrain merged commit 2bf0c2e into master Apr 20, 2021
@achingbrain achingbrain deleted the fix/specify-pbjs-root branch April 20, 2021 11:13
achingbrain added a commit to libp2p/js-libp2p-kad-dht that referenced this pull request Apr 20, 2021
To prevent namespace conflicts with protobuf message names.

Refs ipfs/js-ipfs-bitswap#323
achingbrain added a commit to ipfs/js-ipfs-unixfs that referenced this pull request Apr 20, 2021
To prevent namespace conflicts with protobuf message names.

Refs ipfs/js-ipfs-bitswap#323
achingbrain added a commit to achingbrain/js-libp2p-noise that referenced this pull request Apr 23, 2021
- Updates all deps to pull in the latest buffer and bl modules
- Specifies a root name for pbjs code gen - message names for pbjs
  generated code are global so specifying a root name scopes them
  to prevent two modules accidentally trampling over each other's
  protobuf code by giving their messages the same name

Refs: ipfs/js-ipfs-bitswap#323

BREAKING CHANGE: buffer@6 dropped support for IE and Safari < 10
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