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

use protobufjs as a devDep only #1166

Merged
merged 1 commit into from
Jun 9, 2022
Merged

use protobufjs as a devDep only #1166

merged 1 commit into from
Jun 9, 2022

Conversation

mrnerdhair
Copy link
Contributor

Currently, protobufjs is used only for type information and running unit tests. Using import type allows it to be demoted from dependencies to devDependencies, reducing this package's footprint significantly (and sidestepping various complaints generated by naive interpretations of the output of npm audit to boot).

Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

Nice change, thank you!

But please note that the runtime dependency still exists in ts-proto generated code (see https://github.com/confio/cosmjs-types/blob/v0.5.0/package.json).

@@ -4,7 +4,7 @@ import { Coin } from "cosmjs-types/cosmos/base/v1beta1/coin";
import { TxBody } from "cosmjs-types/cosmos/tx/v1beta1/tx";
import { Any } from "cosmjs-types/google/protobuf/any";
import Long from "long";
import protobuf from "protobufjs";
import type protobuf from "protobufjs";
Copy link
Member

@webmaster128 webmaster128 Jun 9, 2022

Choose a reason for hiding this comment

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

Nice one, did not know this 💎

@webmaster128 webmaster128 merged commit 06a16de into cosmos:main Jun 9, 2022
@mrnerdhair mrnerdhair deleted the protobufjs-as-dev-dep branch June 9, 2022 06:57
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