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

🐛 Spec files included in npm packages #2221

Closed
charkour opened this issue May 8, 2023 · 6 comments
Closed

🐛 Spec files included in npm packages #2221

charkour opened this issue May 8, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@charkour
Copy link

charkour commented May 8, 2023

Describe the bug
.spec.ts files are included in the packaged versions on NPM. I understand that /src files are included for easy reference, but including the test files simply bloats the size of the package on the disk. Is there a compelling reason to continue to keep .spec.ts in the package NPM versions of the packages?

To Reproduce
Steps to reproduce the behavior:

  1. NPM JS and search for @datadog/browser-rum or @datadog/browser-logs
  2. Click on try on RunKit
  3. View source files
  4. See the .spec.ts files included
Screenshot from RunKit showing that `.spec.ts` files are included

image

Expected behavior
Do not include .spec.ts files in the packaged /src because it bloats the download size of the package from NPM. We need to be conscious of the fact that disk space is finite and including bloat is wasteful.

Thank you.

@charkour charkour added the bug Something isn't working label May 8, 2023
@charkour
Copy link
Author

charkour commented May 8, 2023

Here is a good conversation about what to include within a package. I would argue that including src/ is also bloat and should be removed as well as removing the test files.

@bcaudan
Copy link
Contributor

bcaudan commented May 9, 2023

Hi @charkour,

We have included the src files in the npm package in response of this issue: #535
But indeed, thanks for pointing that, specs file should not be necessary, we will remove them.

@charkour
Copy link
Author

charkour commented May 9, 2023

Hi @bcaudan, thanks for the quick reply!

I appreciate linking the past issue. Because source maps are included, we then need to bundle the src/ directory, correct? (I'm not super familiar with source maps)

@bcaudan
Copy link
Contributor

bcaudan commented May 10, 2023

I appreciate linking the past issue. Because source maps are included, we then need to bundle the src/ directory, correct?

Exactly!

About spec files, cf #2224, they should be excluded in the next release.

@bcaudan bcaudan closed this as completed May 10, 2023
@charkour
Copy link
Author

Thanks for the quick update!

@BenoitZugmeyer
Copy link
Member

It's been released as part of v4.42.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants