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

Don't commit node_modules/ #138

Closed
brettatoms opened this issue May 3, 2020 · 9 comments · Fixed by #184
Closed

Don't commit node_modules/ #138

brettatoms opened this issue May 3, 2020 · 9 comments · Fixed by #184
Labels

Comments

@brettatoms
Copy link

By default this project creates an initial commit which includes node_modules/. It generally isn't recommended to commit node_modules/ to your repo especially now that package-lock.json/yarn.lock files for reproducible builds are common.

@filipesilva
Copy link
Owner

Heya @brettatoms, sorry it took so long for me to get back to you.

The initial commit should include this gitignore:

node_modules
public/js
out
.shadow-cljs
.nrepl-port
.clj-kondo/.cache
yarn-error.log
report.html

This should ignore node_modules, but I wonder if I'm missing something here due to my local setup.

@jackdbd
Copy link

jackdbd commented Jun 27, 2020

npx create-cljs-app my-app creates an initial commit with no .gitignore, so all node-modules are committed.

@filipesilva
Copy link
Owner

@jackdbd @brettatoms I'm having a lot of difficulty reproducing what you see.

This is what I see on a windows machine using git bash:

kamik@RED-X1C6 MINGW64 ~
$ npx create-cljs-app my-app-test

Creating a new CLJS app in C:\Users\kamik\my-app-test.

Installing packages. This might take a couple of minutes.
√ Packages installed successfully.

Initialized a git repository.

Success! Created my-app-test at C:\Users\kamik\my-app-test
Inside that directory, you can run several commands:

  yarn start
    Starts the development server.

  yarn build
    Bundles the app into static files for production.

  yarn test
    Starts the test runner.

We suggest that you begin by typing:

  cd my-app-test
  yarn start

Happy hacking!


kamik@RED-X1C6 MINGW64 ~
$ cat my-app-test/.gitignore
node_modules
public/js
out
.shadow-cljs
.nrepl-port
.clj-kondo/.cache
yarn-error.log
report.html

And this is what I see on a Ubuntu install via WSL2:

~/sandbox [127] $ npx create-cljs-app my-app-test

Creating a new CLJS app in /home/filipesilva/sandbox/my-app-test.

Installing packages. This might take a couple of minutes.
✔ Packages installed successfully.

Initialized a git repository.

Success! Created my-app-test at /home/filipesilva/sandbox/my-app-test
Inside that directory, you can run several commands:

  yarn start
    Starts the development server.

  yarn build
    Bundles the app into static files for production.

  yarn test
    Starts the test runner.

We suggest that you begin by typing:

  cd my-app-test
  yarn start

Happy hacking!

~/sandbox $ cat ./my-app-test/.gitignore
node_modules
public/js
out
.shadow-cljs
.nrepl-port
.clj-kondo/.cache
yarn-error.log
report.html

On both cases I see a .gitignore file. But you don't seem to get it, and I'm not sure why.

What OS are you on? Maybe that's related.

@jackdbd I see your PR #146, but I don't understand why that would address the problem since it's removing a lot of ignores but not .gitignore.

@brettatoms
Copy link
Author

@filipesilva This is what I'm seeing on Mac 10.15.5. The contents of your .gitignore does match my .npmignore:

➜ node --version  
v10.18.1

➜ npx --version      
6.13.4

➜ npx create-cljs-app my-app
npx: installed 47 in 6.384s

Creating a new CLJS app in /private/tmp/my-app.

Installing packages. This might take a couple of minutes.
✔ Packages installed successfully.

Initialized a git repository.

Success! Created my-app at /private/tmp/my-app
Inside that directory, you can run several commands:

  yarn start
    Starts the development server.

  yarn build
    Bundles the app into static files for production.

  yarn test
    Starts the test runner.

We suggest that you begin by typing:

  cd my-app
  yarn start

Happy hacking!


/tmp took 2m 41s 
➜ cd my-app

/tmp/my-app on  master is 📦 v0.1.0 via ⬢ v10.18.1 
➜ cat .gitignore 
[bat error]: '.gitignore': No such file or directory (os error 2)

@jackdbd
Copy link

jackdbd commented Jul 30, 2020

I'm on Xubuntu 18.04. When I run npx create-cljs-app my-app I get an .npmignore file, but no .gitignore.

Maybe there is something strange with list-files-helper and the paths from template-ignores are not ignored, so the node_modules directory get copied in my-app.

Here is the content of my .npmignore:

node_modules
public/js
out
.shadow-cljs
.nrepl-port
.clj-kondo/.cache
yarn-error.log
report.html

@brettatoms
Copy link
Author

Could it be that you're calling git add -A which add all files in the folder? https://github.com/filipesilva/create-cljs-app/blob/master/src/create_cljs_app/lib.cljs#L43

@filipesilva
Copy link
Owner

filipesilva commented Aug 24, 2020

I think I understand what's happening. Npm is converting the .gitignore to .npmignore as described in npm/npm#3763.

I must have something odd in my testing setup that didn't run into this behaviour, maybe I globally installed from a dev version and thus didn't get the published version.

At any rate the solution is to add a custom .npmignore with explicit ignores use gitignore instead and rename after creating to .gitignore.

filipesilva added a commit that referenced this issue Oct 4, 2020
filipesilva added a commit that referenced this issue Oct 4, 2020
filipesilva added a commit that referenced this issue Oct 4, 2020
@filipesilva
Copy link
Owner

Heya, I believe this is fixed in create-cljs-app@0.4.1, released a minute or so ago. It should be picked up automatically when doing project creation.

@github-actions
Copy link

github-actions bot commented Oct 4, 2020

🎉 This issue has been resolved in version 0.4.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging a pull request may close this issue.

3 participants