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: actually exclude test files from published release #350

Merged
merged 2 commits into from
Nov 15, 2023

Conversation

legobeat
Copy link
Contributor

@legobeat legobeat commented Nov 14, 2023

Fixes package metadata to actually omit test files from published releases.

Before

$ npm publish --dry-run

> @metamask/[email protected] prepack
> ./scripts/prepack.sh

+ set -e
+ set -o pipefail
+ [[ -n '' ]]
+ yarn build:clean
npm notice
npm notice 📦  @metamask/[email protected]
npm notice === Tarball Contents ===
npm notice 739B    LICENSE
npm notice 5.0kB   README.md
npm notice 2.3kB   dist/encryption.d.ts
npm notice 9.0kB   dist/encryption.js
npm notice 12.5kB  dist/encryption.js.map
npm notice 11B     dist/encryption.test.d.ts
npm notice 12.1kB  dist/encryption.test.js
npm notice 19.0kB  dist/encryption.test.js.map
npm notice 146B    dist/index.d.ts
npm notice 1.3kB   dist/index.js
npm notice 384B    dist/index.js.map
npm notice 11B     dist/index.test.d.ts
npm notice 2.0kB   dist/index.test.js
npm notice 1.5kB   dist/index.test.js.map
npm notice 1.8kB   dist/personal-sign.d.ts
npm notice 3.6kB   dist/personal-sign.js
npm notice 5.2kB   dist/personal-sign.js.map
npm notice 11B     dist/personal-sign.test.d.ts
npm notice 9.1kB   dist/personal-sign.test.js
npm notice 13.7kB  dist/personal-sign.test.js.map
npm notice 9.6kB   dist/sign-typed-data.d.ts
npm notice 24.6kB  dist/sign-typed-data.js
npm notice 42.5kB  dist/sign-typed-data.js.map
npm notice 11B     dist/sign-typed-data.test.d.ts
npm notice 227.5kB dist/sign-typed-data.test.js
npm notice 339.4kB dist/sign-typed-data.test.js.map
npm notice 2.2kB   dist/utils.d.ts
npm notice 4.1kB   dist/utils.js
npm notice 6.1kB   dist/utils.js.map
npm notice 11B     dist/utils.test.d.ts
npm notice 6.2kB   dist/utils.test.js
npm notice 11.3kB  dist/utils.test.js.map
npm notice 3.0kB   package.json
npm notice === Tarball Details ===
npm notice name:          @metamask/eth-sig-util
npm notice version:       7.0.0
npm notice filename:      @metamask/eth-sig-util-7.0.0.tgz
npm notice package size:  79.3 kB
npm notice unpacked size: 776.0 kB
npm notice shasum:        409e6ae979f0baa5cf06f2bdefe17e23835c7893
npm notice integrity:     sha512-SzkWf09ohe2M+[...]k1ACpIf1UN8tw==
npm notice total files:   33
npm notice
+ @metamask/[email protected]

After

$ npm publish --dry-run

> @metamask/[email protected] prepack
> ./scripts/prepack.sh

+ set -e
+ set -o pipefail
+ [[ -n '' ]]
+ yarn build:clean
npm notice
npm notice 📦  @metamask/[email protected]
npm notice === Tarball Contents ===
npm notice 739B   LICENSE
npm notice 5.0kB  README.md
npm notice 2.3kB  dist/encryption.d.ts
npm notice 9.0kB  dist/encryption.js
npm notice 12.5kB dist/encryption.js.map
npm notice 146B   dist/index.d.ts
npm notice 1.3kB  dist/index.js
npm notice 384B   dist/index.js.map
npm notice 1.8kB  dist/personal-sign.d.ts
npm notice 3.6kB  dist/personal-sign.js
npm notice 5.2kB  dist/personal-sign.js.map
npm notice 9.6kB  dist/sign-typed-data.d.ts
npm notice 24.6kB dist/sign-typed-data.js
npm notice 42.5kB dist/sign-typed-data.js.map
npm notice 2.2kB  dist/utils.d.ts
npm notice 4.1kB  dist/utils.js
npm notice 6.1kB  dist/utils.js.map
npm notice 3.0kB  package.json
npm notice === Tarball Details ===
npm notice name:          @metamask/eth-sig-util
npm notice version:       7.0.0
npm notice filename:      @metamask/eth-sig-util-7.0.0.tgz
npm notice package size:  30.3 kB
npm notice unpacked size: 134.2 kB
npm notice shasum:        ed61e5816e44286388f3b95cc4203ea968f6cd1d
npm notice integrity:     sha512-TZwGNse6ppt/U[...]VQE+EaaOr5hcw==
npm notice total files:   18
npm notice
+ @metamask/[email protected]

@mcmire
Copy link

mcmire commented Nov 14, 2023

In the module template we don't have to exclude test files, we just say whatever's in dist gets published: https://github.com/MetaMask/metamask-module-template/blob/023079ea0b60d4bdeb883d4b549f0e3cdec85936/package.json#L26

So that begs the question: why are test files even showing up in dist in the first place? It looks like TypeScript isn't configured the same way that we've configured in the module template. There we actually have two tsconfigs, one for development and another for releases. The release version excludes test files: https://github.com/MetaMask/metamask-module-template/blob/023079ea0b60d4bdeb883d4b549f0e3cdec85936/tsconfig.build.json

So, what do you think about changing the TypeScript config instead?

@legobeat
Copy link
Contributor Author

So, what do you think about changing the TypeScript config instead?

I'd approve such a PR in favor of this one but unless you or someone else is about to do just that, shouldn't we address the existing configuration bug in the meantime?

@mcmire
Copy link

mcmire commented Nov 15, 2023

Sure, I guess that would be a bigger change. I imagine there are other standardization tasks we need to make here anyway.

Copy link

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

LGTM.

@legobeat legobeat merged commit ef18b9d into MetaMask:main Nov 15, 2023
16 checks passed
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