Fail hard if incompatible options are supplied with --webpack

[gauravtiwari]

Summary

• Fail hard if using --webpack and --api option together, they simple aren't made for each other 😄 . Addresses this - Ditching the Rails view layer webpacker#276
• Fail hard if using --webpack and --skip-bundle option together, chicken and egg situation apparently 😄 . Addresses this - rails new does not work with --webpack and --skip-bundle on Rails 5.1 #28916
• Add elm to installer list

// cc @rafaelfranca @dhh

[rafaelfranca]

Where webpacker uses action view? One of the uses cases for webpacker is exacly API applications that want to use client side frameworks.

[gauravtiwari]

@rafaelfranca Webpacker has view helpers like javascript_pack_tag etc. which won't work without views. I guess --api skips that right?

[gauravtiwari]

We can add support to make it work independently without Rails views though 👍

[rafaelfranca]

The only way of using webpacker is with javascript_pack_tag? Is it not possible to user webpacker and don't use those helpers?

[gauravtiwari]

It is possible to use, but there is no out of the box support like helpers to link those webpack bundles. If someone wants to add this they have to install HTML plugin and add some changes to make this work

[gauravtiwari]

The gem does two things - one is using Webpack to compile modern javascript code and two involves helpers to fetch and link them within Rails views. Perhaps with api option supplied together with webpack we can re-structure bits of webpacker gem so that it serve files directly from public directory.

[gauravtiwari]

Sorry for such yarn travis dance. Seems like it simply doesn't work on travis with container-builds - https://yarnpkg.com/lang/en/docs/install-ci/#travis-tab.

[maclover7]

This seems like a duplication of the check_node and check_yarn tasks in webpacker. Is the issue that rails new isn't failing the right way when webpacker is checking dependencies?

[gauravtiwari]

@maclover7 That's correct. I think with --webpack option supplied rails new should fail hard. At the moment it goes up to the point of running webpacker:install and fails there, which isn't great because then the whole app structure is in place and you have to manually run webpacker:install or delete the app and re-run rails new. What do you think?

[gauravtiwari]

Webpacker check is useful for independent gem install. Obviously, one downside is with rails new --webpack we are checking this twice.

[maclover7]

Hmm, spent some time digging around the docs for generators, and may have found a solution. Can you wrap the entire webpacker installer template and the JS dependency checks in an after_bundle callback? I think that would ensure it's the last thing that runs, so it wouldn't bounce people out early from rails new.

[gauravtiwari]

@maclover7 It says in the doc that after_bundle callback gets executed even if --skip-bundle and/or --skip-spring has been passed. Don't you think that will lead to the same issue we are having now? i.e. it will try to run webpacker:install anyway unless I am missing something :)

[maclover7]

@gauravtiwari Sorry, I didn't fully say what I was thinking 😞 How about we keep the webpacker + skip bundle stuff in the app generator (this is more a concern of rails than webpacker), and then move the dependency stuff and the install task to the after_bundle hook.

[gauravtiwari]

@maclover7 No worries :) after_bundle looks great although I am not sure how it would help if say yarn isn't installed on the system and then rails new generates the project structure without failing until webpacker:install. Do you think we shouldn't fail if yarn or node isn't installed and --webpack option is supplied inside app generator?

[gauravtiwari]

@rafaelfranca @maclover7 Removed the yarn and node check part. Good enough for merge?

[maclover7]

@gauravtiwari Since we aren't doing raising for bad versions, where is that happening now? Within webpacker? Is that out of scope now for this PR?

[gauravtiwari]

@maclover7 Yepp that will happen inside webpacker - seems like best thing to do at this point.

[maclover7]

Can you squash down to one commit?

[gauravtiwari]

@maclover7 Done!

Looks like rb-notify was force pushed or something because I was getting this error on initial bundle fatal: Could not parse object '90553518d1fb79aedc98a3036c59bd2b6731ac40' so bundled it to latest revision.

[rafaelfranca]

Should we fail in this case or just skip the wepacker:install call that we do later? I always generate my applications with --skip-bundler (it is in my ~/.railsrc) and I'm fine with adding webpacker in my gemfile when using --webpack and running webpacker:install manually. But the fact that I can't do that when we merge this code annoys me.

[rails-bot]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.