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

yarn_install task is not installing devDependencies #1212

Closed
nikolalsvk opened this issue Jan 24, 2018 · 32 comments
Closed

yarn_install task is not installing devDependencies #1212

nikolalsvk opened this issue Jan 24, 2018 · 32 comments

Comments

@nikolalsvk
Copy link

nikolalsvk commented Jan 24, 2018

Due to the recent changes that have been made, webpacker:yarn_install task is not installing devDependencies. This is breaking my CI builds, since I relied on webpacker to do the install of packages for my test environment on the CI server.

Change that brought this in the webpacker gem d5fe0fe#diff-5a0f30530a2f1282593526bb85d65029
Change that brought this in the Rails gem
rails/rails@2118c98#diff-dcd44149ba1cd25c12956a3ebecc32a3

I'm just wondering if we could introduce an optional --production=true|false flag in the rake task that does yarn install?

Or create a NODE_ENV dependant flag for the rake task?

@nikolalsvk nikolalsvk changed the title yarn_install task is not intalling devDependencies yarn_install task is not installing devDependencies Jan 24, 2018
@gauravtiwari
Copy link
Member

@nikolalsvk Perhaps you can use yarn directly instead of relying on rake task?

@nikolalsvk
Copy link
Author

@gauravtiwari yes, I'm doing yarn install after assets precompilation, but that is just one extra yarn install to do.

I'm just wondering could there be a different yarn_install task for testing environment for example?

@gauravtiwari
Copy link
Member

@nikolalsvk I guess what you could do is disable webpacker rake task and create a new one as before:

# lib/tasks/yarn_install.rake
Rake::Task['yarn_install'].clear

task 'yarn_install' do
    # old task here
end

Would this work?

@nikolalsvk
Copy link
Author

@gauravtiwari guess it would work, thanks.

I'm just curious why not leave the choice whether to install or not to install devDependencies?

@cipater
Copy link

cipater commented Feb 2, 2018

I'm having issues with this change as well. In particular, yarn install --production seems to be removing the babel-plugin-relay, but it's required as part of the asset precompilation... which is now failing due to being unable to find the plugin.

@arthow4n
Copy link

arthow4n commented Feb 6, 2018

IMHO lots of people install webpack-* and babel-* in devDependencies since they are for building instead of serving things. yarn install --production can cause devDependencies being used to build removed. I always perform yarn before doing assets:precompile but now I get my webpack-chunk-hash in devDependencies missing after webpacker:yarn_install after upgrading webpacker. I don't think it is a good idea forcing yarn install --production in webpacker rake task.

@catskull
Copy link

catskull commented Mar 8, 2018

Maybe I'm doing something wrong, but this change broke my production deploys. I too had all my compilation related packages in devDependencies, so the production server couldn't build the assets correctly.

@odlp
Copy link
Contributor

odlp commented Mar 9, 2018

Cross-linking to #1330, where the task ends up removing dev dependencies which have already been installed.

@luccasmaso
Copy link

luccasmaso commented Apr 11, 2018

Any solutions? As in rails 5.2 the --production is harcoded, should I migrate all devDependencies to dependencies? What would be the least worst patch to it?

@groksrc
Copy link

groksrc commented Apr 11, 2018

I am also having issues with --production being hardcoded. I have a lot of things in devDependencies that don't belong in production and I am more likely to remove webpacker than I am to change this. I also have more than just production and development environments and would like more flexibility in passing options to NODE_ENV. It's fine if webpacker makes certain assumptions for me but it shouldn't force me to do things a certain way.

@leikind
Copy link

leikind commented Apr 27, 2018

I have a Rails 5.1.6 app with webpacker successfully deployed on Heroku app with 2 buildpacks:
(1)nodejs and (2) ruby.

Upgrading to 5.2.0 breaks the Heroku build for the reasons @nikolalsvk listed above. The build fails because there is a number of devDependencies not installed after rake yarn:install.

Moving devDependencies to dependencies feels very very wrong.

What is interesting is Heroku's approach to handling devDependencies and dependencies in their nodejs buildpack:

By default, Heroku will install all dependencies listed in package.json under dependencies and devDependencies.

After running the installation and build steps Heroku will strip out the packages declared under devDependencies before deploying the application.

( https://devcenter.heroku.com/articles/nodejs-support#package-installation )

It seems like a more reasonable approach than just excluding all devDependencies.

Unfortunately, the rails buildpack is executed after the nodejs buildpack, and webpacker:yarn_install makes sure none of my devDependencies are present.

I am wondering what the correct solution should be.

@gauravtiwari
Copy link
Member

@leikind They are not devDependencies really because you are using them on Heroku as well, which is a production environment. If however, you compile your assets locally and then push them to Heroku then they can safely live under dev dependencies, considering assets:precompile is never run on Heroku.

With webpacker, we recommend putting all build and app related dependencies under dependencies section in package.json (except test deps). I understand the confusion but you see they are not app dependencies i.e. it won't be included in your pack but they are needed to produce a pack or build - either on Heroku or on your local machine. Does this make sense?

@leikind
Copy link

leikind commented Apr 30, 2018

The app is indeed built on Heroku, assets:precompile is executed.

You see, if my app were a Node.js app using only the node buildpack, I'd have no issues building my app on Heroku, as the node buildpack would install all packages first, run build scripts (webpack for instance), and then strip devDependencies. So, the node team in Heroku does invite its customers to build their node apps on Heroku.

I see no argument against building my app on Heroku except for the webpacker team making it harder.

Here's my current hack to solve my problem:

namespace :yarn do
  desc 'Install all JavaScript dependencies as specified via Yarn'
  task :install do
    system('./bin/yarn install --no-progress')
    Rake::Task['yarn:install'].clear
  end
end

@mhfen
Copy link

mhfen commented Sep 12, 2018

@leikind They are not devDependencies really because you are using them on Heroku as well, which is a production environment. If however, you compile your assets locally and then push them to Heroku then they can safely live under dev dependencies, considering assets:precompile is never run on Heroku.

Many teams run assets:precompile on heroku. In fact, it is a standard part of the rails build pack...

With webpacker, we recommend putting all build and app related dependencies under dependencies section in package.json (except test deps). I understand the confusion but you see they are not app dependencies i.e. it won't be included in your pack but they are needed to produce a pack or build - either on Heroku or on your local machine. Does this make sense?

It does not. Webpack, babel, etc are dev ops tools. They are not application code. Heroku installs devDeps, so why force everyone to make a bad organization decision and move those to deps?

This should be addressed by the Webpacker team...

@richseviora
Copy link

With webpacker, we recommend putting all build and app related dependencies under dependencies section in package.json (except test deps). I understand the confusion but you see they are not app dependencies i.e. it won't be included in your pack but they are needed to produce a pack or build - either on Heroku or on your local machine. Does this make sense?

@gauravtiwari I agree this makes sense if you look at the devDependencies / dependencies distinction as code that's used only locally during the dev workflow (like Prettier, ESLint, etc) vs code that's executed on production. I think what you're hearing from other people (and please count myself in that list) is that they all have different understandings of the term and some of those understandings have been implemented into widely used tools (like Heroku).

I ran into the same problem myself when deploying this via the opsworks_ruby cookbook to AWS. My solution was to move the devDependencies into dependencies, but I'd like to see it install dependencies preferably by default, or as an option.

@yvbeek
Copy link
Contributor

yvbeek commented Nov 27, 2018

This is really confusing. I have the following devDependencies:

"devDependencies": {
  "@types/bootstrap": "<4",
  "@types/braintree-web": "^3.6.3",
  "@types/chartist": "^0.9.43",
  "@types/daterangepicker": "<3",
  "@types/dropzone": "^5.0.4",
  "@types/jquery": "<3",
  "@types/jqueryui": "^1.12.6",
  "@types/moment-timezone": "^0.5.9",
  "caniuse-lite": "^1.0.30000910",
  "node-sass": "^4.10.0",
  "resolve-url-loader": "^3.0.0",
  "sass-loader": "^7.1.0",
  "ts-loader": "^5.3.0",
  "tslint": "^5.11.0",
  "typescript": "^3.1.6",
  "webpack": "^4.26.1",
  "webpack-bundle-analyzer": "^3.0.3",
  "webpack-dev-server": "^3.1.10"
}

It doesn't make sense to me to move all the TypeScript typings and Webpack loaders in dependencies just to make compilation work.

Would it be possible to remove the Yarn install step or turn it off?

@jakeNiemiec
Copy link
Member

jakeNiemiec commented Nov 27, 2018

@leikind @Zyphrax It doesn't make sense to me to move all the TypeScript typings and Webpack loaders in dependencies just to make compilation work.

devDependencies has more usefulness in the context of a JS library.

Let's take webpack as an example. Notice that react is installed under devDependencies.

  • If your developing for the webpack library locally, it would install react.
  • If your developing for a rails app that lists webpack as a dependency, it would NOT install react. This is because webpack does not need react to function, therefore it does not need to pass the lib downstream.

This is what @gauravtiwari was getting at. Your rails app is "the end of the JS line", you don't need to worry about downstream users. I am aware that many READMEs will tell you to use --save-dev, but devDependencies offer no practical use. (esp. in the context of webpacker)

TL;DR: If you need something to build you production bundle, it cannot be placed in devDependencies. (including: webpack, typescript, node-sass, sass-loader)

@chevinbrown
Copy link

@jakeNiemiec is there some confusion here about what devDependencies vs production dependencies means? I feel like many believe that the dependencies include something in a production bundle and therefore increase the package size? (if this question/comment is just chatter, thumbs down and I'll delete)

@odlp
Copy link
Contributor

odlp commented Nov 27, 2018

IMO there are legitimate use cases for devDependencies in the package.json of a Rails app - say you want to have JS testing libraries like Karma, Jasmine etc installed via yarn on CI but don't want these when in the production environment (see #1330 for further context).

Webpacker hardcoding --production into a Rake task is disruptive of other yarn uses which don't relate to asset compilation.

@jakeNiemiec
Copy link
Member

@odlp IMO there are legitimate use cases for devDependencies in a Rails app

You never need to worry about downstream dependencies in a Rails app, if you come across a --save-dev in a README, that was not put there for you (the same problem exists in Node.js-land). @nikolalsvk, What is the purpose for a CI test? To simulate a production environment and programmatically test a bunch of edge cases. Wouldn't you want to test against minified/transpiled production bundles?

say you want to have JS testing libraries like Karma, Jasmine etc installed via yarn on CI but don't want these when in the production environment (see #1330 for further context).

Are you using webpack to bundle Karma or Jasmine? I don't understand why you would do this.

@chevinbrown Is there some confusion here about what devDependencies vs production dependencies means? I feel like many believe that the dependencies include something in a production bundle and therefore increase the package size?

Less "confusion" and more like ritual inclusion of programing patterns that subvert their original purpose (e.g.: running CI tests in development mode). All it does is tell yarn/npm what libraries to put in node_modules. If you include Karma in dependencies, you will just additionally have Karma and it's dependencies in your node_modules. At worse you have an extra 8 seconds to wait when you install your dependencies.

TL;DR: You should follow what @gauravtiwari said, there are mechanical differences between the production and development flags, they are more than just labels:
image

@odlp
Copy link
Contributor

odlp commented Nov 27, 2018

@jakeNiemiec sorry if I was unclear, but to clarify:

Are you using webpack to bundle Karma or Jasmine? I don't understand why you would do this.

I'm using yarn to install Karma and Jasmine. webpacker also happens to use yarn to install dependencies, but it hardcodes --production in it's rake tasks which interferes with non-weback(er) related tasks.

My suggestion is the webpacker commands should be environment aware.

@jakeNiemiec
Copy link
Member

jakeNiemiec commented Nov 28, 2018

@odlp My suggestion is the webpacker commands should be environment aware.

image

  • Think of it like this, if you really wanted to, you could hit a nail with the non-flat side of a hammer...it would work, but you would be more prone to problems.

  • In the same way setting NODE_ENV comes with side-effects specific to your dependencies and the dependencies of those dependencies.

If you pass production bundles to your tests, you will get less surprises, faster testing times, and results that are truer to your actual production env. (but at the cost of increased build time)

Real-life example: The Font Awesome package tends to break when using webpack 4, but only if you are in production mode.

However, if you really want to have the autonomy to do this, you can use this trick to force a different environment: NODE_ENV=development karma start ./config/karma.conf.js or however you are running your tests, just set or unset NODE_ENV before the relevant command. If you are still missing karma from your CI server, you can globally add it with: yarn global add karma. If you do use this, please note that this has its own perils and required reading. Best of luck!

@trent-boyd
Copy link

@gauravtiwari

With webpacker, we recommend putting all build and app related dependencies under dependencies section in package.json (except test deps).

That's an example of bikeshedding if I ever saw one. Further, this is listed nowhere in the documentation. I should not have to crawl the Webpacker source or track down this exact issue just to find out that your team is imposing their own view on how devDependencies should work.

Hundreds of other JS-based open source projects intentionally instruct you to use the --dev flag during installation. Webpacker is a development tool for bundling up code intended to be run in a browser. It would make sense to me that other development tools (like Babel, its plugins, and other Webpack loaders) would also be installed as development dependencies.

And besides that, what is the --production flag really helping anyway? If I want to really streamline my NPM install times or cut down on disk usage, that should be something I opt-in to.

@ytbryan
Copy link
Contributor

ytbryan commented Nov 29, 2018

I should not have to crawl the Webpacker source or track down this exact issue just to find out that your team is imposing their own view on how devDependencies should work.

@trent-boyd Mr Boyd, please feel free you should to send a PR

Thank you @gauravtiwari for your time contributing to the rails/webpacker project. Webpacker has been a delight and it saves me so much time integrating Vue into Rails <3

@trent-boyd
Copy link

I don't think you'd like the the voice of my documentation on this one. 😉

It's just frustrating to see a lack of attention to his, especially when dev team has come up against it themselves and there's been an unaddressed PR about it for nine months. Even closing something as a #wontfix is fine if that's what the maintainers decide.

@jakeNiemiec
Copy link
Member

@trent-boyd It's just frustrating to see a lack of attention to his, especially when dev team has come up against it themselves

The dev team has come up against it? Pretty sure the resolution was the opposite of what you portray:

Yes let's fix the root issue and move away from errant use of devDependencies.

That's an example of bikeshedding if I ever saw one. I should not have to [...] find that your team is imposing their own view on how devDependencies should work.

👆That's bikeshedding if I ever saw it. The WebpackER team is aligning themselves with how Webpack is used by the Node.js community. This is smart, it will do a lot to reduce friction between Rails & JS pipelines.

Remember that package.json & devDependencies are all constructs from the Node.js world. Nothing is being imposed, it's just being aligned with existing usage/documentation.

Hundreds of other JS-based open source projects intentionally instruct you to use the --dev flag during installation.

Like I said here, those instructions aren't always meant for end users that are not creating a package themselves (like any Webpacker app). The 2 links below are useful for explaining why this is:
https://yarnpkg.com/blog/2018/04/18/dependencies-done-right/
https://stackoverflow.com/a/22004559/6091685

Webpacker is a development tool for bundling up code intended to be run in a browser. It would make sense to me that other development tools (like Babel, its plugins, and other Webpack loaders) would also be installed as development dependencies.

Rails is a development tool too 🙃. Did you know that both Webpack and Babel are set up to behave differently depending on which flag is set? (NODE_ENV) I personally would want to run tests against production bundles.

The webpacker team is just trying to keep you on the most pain-free path. Just chuck your devDependencies in dependencies and be done with it...none of this will matter soon anyway because the rules are changing ¯\(ツ).

@trent-boyd
Copy link

Yes let's fix the root issue and move away from errant use of devDependencies.

Neat. Awesome. I'm on board as long as it's in the docs.

Did you know that both Webpack and Babel are set up to behave differently depending on which flag is set? (NODE_ENV). I personally would want to run tests against production bundles.

Yes, and that's how I'd prefer it to stay. Using the --production flag bypasses that. And the test guy is someone else, but I do agree with you on that.

@gauravtiwari
Copy link
Member

Thanks, everyone and sorry about the confusion 🙏 @jakeNiemiec really like the way you have explained it ❤️

@trent-boyd I understand this could be confusing but even if we remove production flag, which I think we should because it hardcodes the environment. Anyway, Yarn would use NODE_ENV to infer the current env and install dependencies accordingly. That's why I suggested putting all build dependencies in the dependencies section because they are needed to build the webpack bundle, unless you are building locally. Of course, you are free to exclude test and linter modules because you won't be using them on production (up to you).

@nikolalsvk
Copy link
Author

We're still having production flag hardcoded y'all and I still don't understand why 😄

@csalvato
Copy link

csalvato commented Jan 9, 2019

Reading this thread, I'm finding myself super confused. Are we supposed to not use devDependencies at all when using Webpacker? Is that the intended use? 😕

We don't use this rake task in our project (we just run yarn install directly), but found that using devDependencies were causing us problems when debugging a discrepancy between dev and prod environments

@catskull
Copy link

catskull commented Jan 9, 2019

We just don't use devDependencies. Yeah extra packages might get downloaded in production but it shouldn't be that big of a deal. To me it's just a paper cut, but the few hours I spent debugging I'll never get back.

@jakeNiemiec
Copy link
Member

jakeNiemiec commented Jan 9, 2019

@csalvato Don't feel bad, people have been confusing this for over a year 2 years:

image

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

No branches or pull requests