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

Add node_modules to .gitignore #107

Closed
mcalthrop opened this issue Dec 7, 2022 · 5 comments
Closed

Add node_modules to .gitignore #107

mcalthrop opened this issue Dec 7, 2022 · 5 comments
Labels
duplicate This issue or pull request already exists

Comments

@mcalthrop
Copy link

Is your feature request related to a problem? Please describe.

Why store node_modules in the repository? This is an anti-pattern.

It also makes it almost impossible to see the difference between releases of this package.

For example:
v9.0.0...v9.1.0

Screenshot 2022-12-07 at 14 37 45

Describe the solution you'd like

The node_modules folder should be added to .gitignore.

Describe alternatives you've considered

N/A

Additional context

N/A

@adamhenson adamhenson added the duplicate This issue or pull request already exists label Dec 7, 2022
@adamhenson
Copy link
Collaborator

adamhenson commented Dec 7, 2022

Trust me, I hate this more than you, since it seems that you haven't had the experience of authoring a JavaScript GitHub Action.

I agree now as I did over two years ago. Welcome to GitHub Actions authoring.

I'm closing this because it's not a specific pattern of this repo, but necessary per the docs in authoring a JavaScript based GitHub Action. Also, this is a duplicate of #32

Related Links

Direct Reference

Creating a JavaScript action - GitHub Docs

GitHub downloads each action run in a workflow during runtime and executes it as a complete package of code before you can use workflow commands like run to interact with the runner machine. This means you must include any package dependencies required to run the JavaScript code. You'll need to check in the toolkit core and github packages to your action's repository.

From your terminal, commit your action.yml, index.js, node_modules, package.json, package-lock.json, and README.md files. If you added a .gitignore file that lists node_modules, you'll need to remove that line to commit the node_modules directory.

Attempt to Use @vercel/ncc

I did try everything in my power to prevent the need to version node_modules, including the one alternative provided by the docs, to implement @vercel/ncc. I ran into several issues seeming like they were bugs from @vercel/ncc. This was years ago, so I don't recall the details, but it was specific to the Lighthouse dependency. Certain parts of the Lighthouse package weren't being exported when using @vercel/ncc if I remember correctly.

@adamhenson
Copy link
Collaborator

adamhenson commented Dec 7, 2022

This is an anti-pattern.

God... is that you? Please stop adding comments like this on any repo.

@mcalthrop
Copy link
Author

Trust me, I hate this more than you, since it seems that you haven't had the experience of authoring a JavaScript GitHub Action.

I agree now as I did over two years ago. Welcome to GitHub Actions authoring.

I'm closing this because it's not a specific pattern of this repo, but necessary per the docs in authoring a JavaScript based GitHub Action. Also, this is a duplicate of #32

Related Links

Direct Reference

Creating a JavaScript action - GitHub Docs

GitHub downloads each action run in a workflow during runtime and executes it as a complete package of code before you can use workflow commands like run to interact with the runner machine. This means you must include any package dependencies required to run the JavaScript code. You'll need to check in the toolkit core and github packages to your action's repository.

From your terminal, commit your action.yml, index.js, node_modules, package.json, package-lock.json, and README.md files. If you added a .gitignore file that lists node_modules, you'll need to remove that line to commit the node_modules directory.

Attempt to Use @vercel/ncc

I did try everything in my power to prevent the need to version node_modules, including the one alternative provided by the docs, to implement @vercel/ncc. I ran into several issues seeming like they were bugs from @vercel/ncc. This was years ago, so I don't recall the details, but it was specific to the Lighthouse dependency. Certain parts of the Lighthouse package weren't being exported when using @vercel/ncc if I remember correctly.

@adamhenson Thanks for taking the time to explain this. What a pain. Didn't see the duplication, apologies.

@adamhenson
Copy link
Collaborator

adamhenson commented Dec 7, 2022

No problem... I get it. And yes, it is disturbing - I'm with you there. Seeing this made me wonder if they've changed that requirement at all since then, or offered a better solution, but it appears not unfortunately.

@mcalthrop
Copy link
Author

No problem... I get it. And yes, it is disturbing - I'm with you there. Seeing this made me wonder if they've changed that requirement at all since then, or offered a better solution, but it appears not unfortunately.

Bummer. Thanks for the GH action though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

2 participants