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

craco should actually be added to the project "dependencies", and not "devDependencies" #23

Merged

Conversation

ndbroadbent
Copy link
Contributor

This is because it is required for production builds, so it always needs to be present. Otherwise, if yarn install or npm install is run in an environment where NODE_ENV is set to production, it won't be installed and the build will fail.

This is also why react-scripts is in the "dependencies" section.

… "devDependencies", because it is required for production builds
@yohanb
Copy link
Contributor

yohanb commented Nov 13, 2018

@ndbroadbent good point but I do find it awkward to npm install or even npm run build on a production environment. Production environments should have a pre-built package of static assets. My understanding is that devDependencies are for build time dependencies and dependencies are for runtime.

@ndbroadbent
Copy link
Contributor Author

ndbroadbent commented Nov 13, 2018

@yohanb Yes that's true for static sites that you can build locally and push up to S3 or something. But if you're pushing to Heroku, Now, Flynn, Convox, etc., these tools have a build process that runs in a container where NODE_ENV is set to production, so it only installs "dependencies" by default. (And if you set it to development, then you install all the stuff you don't need like eslint and prettier.)

I found all these issues on create-react-app about dependencies vs. devDependencies:

I think the most important thing is to just follow create-react-app and put craco in the same place as react-scripts. And it will save some headaches for people who deploy their app to Heroku, etc.

@patricklafrance
Copy link
Contributor

@ndbroadbent I haven't thought about how those providers works, thank you!

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

Successfully merging this pull request may close these issues.

3 participants