MadeInHaus / react-starter

A client-side SPA starter bundled with React, Redux, CSS Modules, React Router 4, and Webpack 4
MIT License
9 stars 2 forks source link

Husky pre-commit hooks don't work with Docker #28

Closed stevescavo closed 6 years ago

stevescavo commented 6 years ago

The Husky pre-commit hooks rely on locally installed node modules. So if you're deploying locally via Docker, the pre-commit hooks fail. Feels "wrong" to be forced to run a local install of dependencies when I'm already running Docker.

Possible solutions:

  1. Make these hooks work when deploying via Docker
  2. Remove them entirely. These hooks are pretty heavy-handed and users (outside of HAUS) might not want these as part of their workflow.

Open to other suggestions.

chrisjcodes commented 6 years ago

I need more context into this issue. Husky should only run when making commits so its a dev dependancy. It is running during deployment? If so that shouldn't be happening. It should only run against staged files when making a commit.

stevescavo commented 6 years ago

Husky should only run when making commits so its a dev dependancy.

Correct. It's doing that now, but it relies on locally installed node_modules. One of the benefits of Docker is that your dependencies are not installed locally, but they reside inside your container. After a quick Google, there appear to be some solutions in place. I don't mind looking more into this when I have some time.

It is running during deployment?

"I'm deploying Docker locally" is synonymous with saying "I'm running Docker locally". Sorry for the confusion.

rynocouse commented 6 years ago

As of now the a host machine must also have node installed to use husky and commit via Tower or any other tool that does not have access to bash exports. This seems to be the one place where we're breaking the encapsulation of a docker running node.

I'd ❤️ to get this working across the board. The pre commit hooks are awesome.

rynocouse commented 6 years ago

Check out the PR #30. Runs (exec) inside the web container

rynocouse commented 6 years ago

Note: This pre commit requires you have to already built the web container at one point