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

All files are included into npm #238

Closed
JounQin opened this issue Mar 23, 2021 · 5 comments
Closed

All files are included into npm #238

JounQin opened this issue Mar 23, 2021 · 5 comments

Comments

@JounQin
Copy link

JounQin commented Mar 23, 2021

No description provided.

@ljharb
Copy link
Member

ljharb commented Mar 23, 2021

This is quite intentional. Those files must be included.

Tests should be published with every package, so that npm explore foo && npm install && npm test always works.

Installed package size is irrelevant; if you never require the test files, they'll never affect the runtime of your application.

Duplicate of #235. Duplicate of #58. Duplicate of #44. See #105 (comment).

@ljharb ljharb closed this as completed Mar 23, 2021
@JounQin
Copy link
Author

JounQin commented Mar 23, 2021

@ljharb

I didn't get the point why npm test should be run inside node_modules.

If you want to test, you can always use git version to install the package.

I raised the PR because when I open the file in VSCode, vscode-eslint reports @ljharb config not found because there is a .eslintrc in the package.

@ljharb
Copy link
Member

ljharb commented Mar 23, 2021

why would you be trying to lint files in node_modules (when not cding into the package directly)? that seems like vscode-eslint is broken.

I can't, in fact, always use the git version, because I might not have internet access when I need to run the tests.

Either way, whether you get the point or not is irrelevant; it's a critical use case for me, and every package I maintain will always publish all needed files to npm, and it's a bug if they don't.

@JounQin
Copy link
Author

JounQin commented Mar 23, 2021

why would you be trying to lint files in node_modules (when not cding into the package directly)? that seems like vscode-eslint is broken.

It's automatically done by vscode-eslint. You can say vscode-eslint is broken, or not. Because there is a .eslintrc, why not read it?

I can't, in fact, always use the git version, because I might not have internet access when I need to run the tests.

But for this package, I suppose you always have the access, didn't you?

Either way, whether you get the point or not is irrelevant; it's a critical use case for me, and every package I maintain will always publish all needed files to npm, and it's a bug if they don't.

Well, again, you're the boss here. I just want say that this package is very popular, and the installation size is also important. I understand your own workflow, that's great. But as you said there are duplicate attempts to resolve this problem, then it's indeed a problem for others, I hope you can consider it more carefully.

@ljharb
Copy link
Member

ljharb commented Mar 23, 2021

I don't find installation size to be that important - it's all tarballed, and untarring it, whether large or small, is basically equally instantaneous.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants