Skip to content
This repository has been archived by the owner on Oct 2, 2019. It is now read-only.

Bring webpack tasks to npm scripts and wrap it with rakes #57

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

halan
Copy link

@halan halan commented Sep 18, 2016

I usually do it in my projects. What you think about?

We can run npm tasks or rake tasks whenever you need:

npm start
npm run build

rake webpack:dev
rake webpack:compile

I often run rails s in one tab and npm start in another.

The main advantage is have all commands configured in just one place and can eventually be removed from rails (without ruby methods as dependency to run webpack server o build it).

ENV["NODE_ENV"] = 'production'
webpack_bin = ::Rails.root.join(::Rails.configuration.webpack.binary)
config_file = ::Rails.root.join(::Rails.configuration.webpack.config_file)
task(:compile) { sh "npm run build" }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We gotta make sure NODE_ENV will be 'production' when this script is run.

@mipearson
Copy link
Owner

I need to think about this one a bit more. I was originally against this because if you changed away from the default paths in your config it'd break.. but there's lots of 👍 's and it's really annoying having to wait for rails to boot just to run a compile.

@@ -2,6 +2,10 @@
"name": "webpack-rails-example",
"version": "0.0.1",
"license": "MIT",
"scripts": {
"build": "webpack -d --config config/webpack.config.js",
"start": "webpack-dev-server -p --config config/webpack.config.js"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to rename start to serve. People's apps might have other things they want to do with npm start that aren't webpack.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

@halan
Copy link
Author

halan commented Sep 21, 2016

Yeah, if you change the default path it will break. I usually put configs into config/js/ but I have only one and simple place to change, the package.json. Who use webpack hope it. Let npm do his job.

@halan
Copy link
Author

halan commented Sep 21, 2016

Look. How can I improve more? @talyssonoc, @mipearson.

@mipearson
Copy link
Owner

I unfortunately can't merge this as it is as it will be a breaking change for those that have reconfigured paths in webpack-rails and expect the rake task to 'just work'.

That said, though, this is the better way.

I will be making a 1.x branch and we can merge it in to that.

@halan
Copy link
Author

halan commented Sep 22, 2016

Nice. Anyway I will think about improve it in this way.

@halan
Copy link
Author

halan commented Sep 22, 2016

What you think about this?:

namespace :webpack do
  desc "Compile webpack bundles"
  task compile: :environment do
    ENV["TARGET"] = 'production' # TODO: Deprecated, use NODE_ENV instead
    ENV["NODE_ENV"] = 'production'

    package_json = JSON.parse(File.read(Rails.root.join('package.json')))

    if( package_json['scripts']['compile'] )
      sh "npm run compile"
    else
      # WE can put a deprecation warning here

      webpack_bin = ::Rails.root.join(::Rails.configuration.webpack.binary)
      config_file = ::Rails.root.join(::Rails.configuration.webpack.config_file)

      unless File.exist?(webpack_bin)
        raise "Can't find our webpack executable at #{webpack_bin} - have you run `npm install`?"
      end

      unless File.exist?(config_file)
        raise "Can't find our webpack config file at #{config_file}"
      end

      sh "#{webpack_bin} --config #{config_file} --bail"
    end
  end
end

@bpinto
Copy link

bpinto commented Oct 18, 2016

This is really cool! Would love to see this as it doesn't require my rails environment (secrets.yml, redis.yml) to exist at the time of assets compilation.

@@ -2,6 +2,10 @@
"name": "webpack-rails-example",
"version": "0.0.1",
"license": "MIT",
"scripts": {
"compile": "webpack -d --config config/webpack.config.js",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you have used d here but you wanted p and the same on the line below.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'r right. I'll fix it. Thanks.

@mipearson
Copy link
Owner

I'm going to be bringing this in to an API-breaking 1.0-pre branch shortly.

The delay on this (and other) pull requests is that I've been cautious to merge anything that could break functionality for existing users.

@halan
Copy link
Author

halan commented Oct 26, 2016

Of course, I understand. Do you need some more adjustments?

@@ -2,6 +2,10 @@
"name": "webpack-rails-example",
"version": "0.0.1",
"license": "MIT",
"scripts": {
"compile": "webpack -d --config config/webpack.config.js",
"serve": "webpack-dev-server -p --config config/webpack.config.js"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When using -p option, I had the following issue: webpack/webpack#1385 which was fixed by removing the -p option.

when using new UglifyJsPlugin and --opimize-minimize (or -p) you are adding the UglifyJsPlugin twice. Omit the CLI option.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. I will change.

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

Successfully merging this pull request may close these issues.

5 participants