Add static assets support #153

gauravtiwari · 2017-03-02T17:15:42Z

Major Changes

Use config/weback/paths.yml and config/weback/dev_server.yml and to pass configuration options to rails and webpack. It seems like this is the best option, because both ruby and javascript has great support for YAML. Removed all configurations from Rails environment file.

# config/webpack/paths.yml
# Restart webpack-watcher or webpack-dev-server if you make changes here
config: config/webpack
entry: bundles
output: public
node_modules: node_modules
source: app/javascript

# config/webpack/dev_server.yml
# Restart webpack-dev-server if you make changes here
enabled: true
host: localhost
port: 8080

// In config/webpack/configuration.js
// Common configuration for webpacker loaded from config/webpack/paths.yml

const { join, resolve } = require('path')
const { env } = require('process')
const { safeLoad } = require('js-yaml')
const { readFileSync } = require('fs')

const configPath = resolve('config', 'webpack')
const paths = safeLoad(readFileSync(join(configPath, 'paths.yml'), 'utf8'))
const devServer = safeLoad(readFileSync(join(configPath, 'dev_server.yml'), 'utf8'))
const publicPath = env.NODE_ENV !== 'production' && devServer.enabled ?
  `http://${devServer.host}:${devServer.port}/` : `/${paths.entry}/`

module.exports = {
  devServer,
  env,
  paths,
  publicPath
}

// In config/webpack/shared.js
const { env, paths, publicPath } = require('./configuration.js')

# Loads webpacker configuration from config/webpack/paths.yml
require "webpacker/file_loader"

class Webpacker::Configuration < Webpacker::FileLoader
  class << self
    def file_path
      Rails.root.join("config", "webpack", "paths.yml")
    end

    def manifest_path
      Rails.root.join(entry_path, "manifest.json")
    end

    def entry_path
      Rails.root.join(paths.fetch(:output, "public"), paths.fetch(:entry, "packs"))
    end

    def paths
      load if Rails.env.development?
      raise Webpacker::FileLoader::FileLoaderError.new("Webpacker::Configuration.load must be called first") unless instance
      instance.data
    end

    def shared_config_path
      Rails.root.join(paths.fetch(:config, "config/webpack"), "shared.js")
    end
  end

  private
    def load
      return super unless File.exist?(@path)
      HashWithIndifferentAccess.new(YAML.load(File.read(@path)))
    end
end

Out-of-the-box support for static assets.

import React from 'react'
import ReactDOM from 'react-dom'
import clockIcon from '../counter/images/clock.png';
import './hello-react.sass'

const Hello = props => (
  <div className="hello-react">
    <img src={clockIcon} alt="clock" />
    <p>Hello {props.name}!</p>
  </div>
)

Use manifest plugin to generate manifest and cleanup webpacker:compile task. Now, manifest.json is used to lookup assets in all environments.

new ManifestPlugin({
    fileName: 'manifest.json',
    publicPath
})

{
  "application.js": "/packs/application-61c89ec4d430372bfdbf.js",
  "clock.png": "/packs/clock-fc31531de5cc3518c7c658d5b83faa72.png"
}

{
  "application.js": "/packs/application.js",
  "application.js.map": "/packs/application.js.map"
}

{
  "app.js": "http://localhost:8080/app.js",
  "app.js.map": "http://localhost:8080/app.js.map"
}

Add a new stylesheet_pack_tag helper tag to helper.rb

 def stylesheet_pack_tag(name, **options)
    stylesheet_link_tag(Webpacker::Manifest.lookup("#{name}#{compute_asset_extname(name, type: :stylesheet)}"), **options)
  end

<%= stylesheet_pack_tag 'hello_react' %>

Add a new asset pack path helper tag to helper.rb

  def asset_pack_path(name, **options)
    asset_path(Webpacker::Manifest.lookup(name), **options)
  end

<%= asset_pack_path 'hello_react', type:  :javascript %>

Minor updates

Update Readme with new changes
Add rubocop from rails repo and lint everything [as per rails setup]
Add rubocop to travis
Filter undefined env variables
Add a separate version file

Demo: https://webpacker-example-app.herokuapp.com/

I made some out-of-scope changes like linting. Since, this involved touching a lot of files, I thought it would make sense to do it now. Sorry :)

@dhh @dleavitt @guilleiguaran Please review when you get a chance ❤️ . Obviously, there might be some things that I have missed and sorry if I trespassed 😄

To try this out locally,

rails new webpacker-app

Open up the gemfile and add

gem 'webpacker', github: 'gauravtiwari/webpacker', branch: 'feature/static-assets'

run bundle install then,

bundle exec rails webpacker:install

# All or one (optional)
bundle exec rails webpacker:install:react
bundle exec rails webpacker:install:angular
bundle exec rails webpacker:install:vue

# To add static assets support
bundle exec rails g controller pages index

In config/routes.rb

root to: 'pages#index'

Open up view pages/index.html.erb

<h1>Pages#index</h1>
<p>Find me in app/views/pages/index.html.erb</p>
<%= javascript_pack_tag 'application' %>
<%= javascript_pack_tag 'hello_react' %>
<hello-angular>Loading...</hello-angular>
<%= javascript_pack_tag 'hello_angular' %>
<%= javascript_pack_tag 'hello_vue' %>
<%= stylesheet_pack_tag 'hello_react' %>

Finally, run webpack-watcher in one terminal window,

./bin/webpack-watcher

and in other,

bundle exec rails s

Go to http://localhost:3000 to see all example apps running. Now, to test out styles,

in app/javascript/packs, create a file called hello_react.sass

.hello-react
  max-width: 500px
  margin: 0 auto

add a className to react example,

const Hello = props => (
  <div className="hello-react">Hello {props.name}!</div>
)

and finally, add <%= stylesheet_pack_tag 'hello_react' %> to pages/index.html.erb

Reload the browser and Voila!

Deploy it to heroku

Replace sqlite gem with pg

# From terminal in order
bundle install
git init && git add . && git commit -am "Webpacker demo"
heroku create 
heroku buildpacks:add --index 1 heroku/nodejs
heroku buildpacks:add --index 2 heroku/ruby
heroku addons:create heroku-postgresql:hobby-dev
git push heroku master

dleavitt · 2017-03-02T17:39:07Z

Awesome! I'd love to give you some thoughts/feedback on this at some point if you're interested!

gauravtiwari · 2017-03-02T17:44:52Z

Sure @dleavitt please do 👍

justin808

Great Start!

Needs documentation that the webpack configs need 2 things:

The magic output directory that the helpers know about.
The use of the ManifestPlugin to creat the magic manifest file.

justin808 · 2017-03-03T05:42:54Z

README.md

-method will automatically insert the correct digest when run in production mode. Just like the asset
-pipeline does it.
+To compile all the packs during deployment, you can use the `rails webpacker:compile` command. This will invoke the production configuration. The `javascript_pack_tag`
+helper method will automatically insert the correct compiled pack in production mode. Just like the asset pipeline does it.



Add the stylesheet_pack_tag docs.

justin808 · 2017-03-03T05:47:50Z

README.md

-will invoke the production configuration, which includes digesting. The `javascript_pack_tag` helper
-method will automatically insert the correct digest when run in production mode. Just like the asset
-pipeline does it.
+To compile all the packs during deployment, you can use the `rails webpacker:compile` command. This will invoke the production configuration. The `javascript_pack_tag`


Do we need to specify the -e production.

To compile all the packs during deployment, you can use the `rails -e production webpacker:compile` command. This will invoke the production configuration.

This will invoke the production configuration.

Maybe remove that sentence.

justin808 · 2017-03-03T05:51:10Z

README.md

-method will automatically insert the correct digest when run in production mode. Just like the asset
-pipeline does it.
+To compile all the packs during deployment, you can use the `rails webpacker:compile` command. This will invoke the production configuration. The `javascript_pack_tag`
+helper method will automatically insert the correct compiled pack in production mode. Just like the asset pipeline does it.


The javascript_pack_tag helper method will automatically insert the correct compiled pack in production mode. Just like the asset pipeline does it.

How about:

The javascript_pack_tag helper method will insert the compiled JavaScript bundle (aka pack) depending on one of 3 environments: production, static development, and hot-reloading for development.

justin808 · 2017-03-03T05:57:01Z

lib/install/bin/webpack-dev-server.tt

@@ -9,19 +9,56 @@ NODE_ENV    = ENV['NODE_ENV']

 APP_PATH = File.expand_path('../', __dir__)
 ESCAPED_APP_PATH = APP_PATH.shellescape
-
 SET_NODE_PATH  = "NODE_PATH=#{ESCAPED_APP_PATH}/node_modules"
 WEBPACKER_BIN  = "./node_modules/.bin/webpack-dev-server"
 WEBPACK_CONFIG = "#{ESCAPED_APP_PATH}/config/webpack/#{NODE_ENV}.js"


WEBPACKER_BIN = "./node_modules/.bin/webpack-dev-server" WEBPACK_CONFIG = "#{ESCAPED_APP_PATH}/config/webpack/#{NODE_ENV}.js"

For React on Rails, if we made this configurable

NODE_MODULES_PATH = "./node_modules" # or from ENV WEPBACK_CONFIG_PATH=""#{ESCAPED_APP_PATH}/config/webpack" # or from ENV WEBPACKER_BIN = "#{NODE_MODULES_PATH}/.bin/webpack-dev-server" WEBPACK_CONFIG = "#{WEBPACK_CONFIG_PATH}/#{NODE_ENV}.js"

justin808 · 2017-03-03T06:01:50Z

lib/install/bin/webpack-dev-server.tt

+        "#{RAILS_ENV_CONFIG}:#{line_num} \n\n"
+      end
+
+      value = value.strip.sub(/https?\:(\\\\|\/\/)(www.)?/,'').gsub('"', '')


value = value.strip.sub(/https?\:(\\\\|\/\/)(www.)?/,'').gsub('"', '')

Consider %r to avoid escaping the /

value = value.strip.sub(%r{https?\:(\\\\|//)(www.)?},'').gsub('"', '')

justin808 · 2017-03-03T07:04:07Z

lib/install/config/shared.js

+  }
+}
+
+if (devHost && devPort) {


All this logic should go into a file that is called like webpack.development.hot.config.

I like to have a file that does simple creation of the assets statically for development mode. Hitting CMD-R has been better for us on big projects.

justin808 · 2017-03-03T07:04:21Z

lib/install/config/shared.js

      }
    ]
  },

  plugins: [
-    new webpack.EnvironmentPlugin(Object.keys(process.env))
+    new webpack.EnvironmentPlugin(Object.keys(process.env)),
+    new ExtractTextPlugin('[name].css'),


move to production file.

justin808 · 2017-03-03T07:13:03Z

lib/webpacker/helper.rb

@@ -15,6 +15,27 @@ module Webpacker::Helper
  #   <%= javascript_pack_tag 'calendar', 'data-turbolinks-track': 'reload' %> # =>
  #   <script src="/packs/calendar-1016838bab065ae1e314.js" data-turbolinks-track="reload"></script>
  def javascript_pack_tag(name, **options)
-    javascript_include_tag(Webpacker::Source.new(name).path, **options)
+    filename, _extension = name.split('.')
+    filename += '.js'


File.basename(x, ".js")

https://ruby-doc.org/core-2.2.0/File.html#method-c-basename

justin808 · 2017-03-03T07:14:19Z

lib/webpacker/helper.rb

+  #   <link rel="stylesheet" media="screen" href="/packs/calendar-1016838bab065ae1e122.css" data-turbolinks-track="reload" />
+  def stylesheet_pack_tag(name, **options)
+    filename, _extension = name.split('.')
+    filename += '.css'


File.basename(x, ".js")

https://ruby-doc.org/core-2.2.0/File.html#method-c-basename

justin808 · 2017-03-03T07:19:14Z

lib/webpacker/source.rb

    else
-      File.join(dist_path, filename)
+      Webpacker::Digests.lookup(filename)


This part is 👍 .

gauravtiwari · 2017-03-03T07:26:58Z

Thanks @justin808 for reviewing. I will make the changes 👍

justin808 · 2017-03-03T07:46:59Z

lib/webpacker/digests.rb

@@ -1,5 +1,5 @@
 # Singleton registry for accessing the digested filenames computed by Webpack in production mode.
-# This allows javascript_pack_tag to take a reference to, say, "calendar.js" and turn it into 
+# This allows javascript_pack_tag to take a reference to, say, "calendar.js" and turn it into
 # "calendar-1016838bab065ae1e314.js". These digested filenames are what enables you to long-term
 # cache things in production.
 class Webpacker::Digests


@gauravtiwari We need to fix the errors in this file so they are crystal clear.

If somebody starts the rails server without first starting, and having the compilation complete, we won't have a digest file.

The typical case is to use a profile to start both the rails server and the dev server in watch mode for generating static files. The rails server will start first, and then there will be no manifest.

Here is an example:
https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/Procfile.static

# Run Rails without hot reloading (static assets). rails: REACT_ON_RAILS_ENV= rails s -b 0.0.0.0 # Build client assets, watching for changes. rails-client-assets: sh -c 'yarn run build:dev:client' # Build server assets, watching for changes. Remove if not server rendering. rails-server-assets: sh -c 'yarn run build:dev:server'

justin808 · 2017-03-05T08:53:37Z

.rubocop.yml

@@ -0,0 +1,124 @@
+AllCops:


awesome to have rubocop!

justin808 · 2017-03-05T08:54:23Z

.travis.yml

@@ -13,10 +13,12 @@ cache:
  yarn: true

 install:
+  - gem install rubocop


shouldn't this get handled by the gemfile?

@justin808 hmm, may be but don't like dependencies. Also, it seems rubocop is more kinda of standalone program now, like yarn and npm

Yes, it should be in the Gemfile. If it is a development dependency, it should be there.

justin808 · 2017-03-05T08:57:22Z

lib/install/bin/webpack-dev-server.tt

-      host, port = value.split(':')
+config = begin
+  package_json = File.read(File.join(APP_PATH, 'package.json'))
+  JSON.parse(package_json)['config']


you may nil if there is no 'config' section

Error message should either mention that package.json is not found or config key is not found.

I would avoid a very generic name like 'config'. Generic is more likely to get a conflict in the future.

How about 'railsConfig'?

justin808 · 2017-03-05T08:58:21Z

lib/install/bin/webpack-dev-server.tt

-  "--config #{WEBPACK_CONFIG} --content-base #{ESCAPED_APP_PATH}/public/packs"
-  " #{ARGV.join(" ")}"
+  exec "NODE_PATH=#{NODE_MODULES_PATH} #{WEBPACK_BIN} " \
+  "--config #{WEBPACK_CONFIG} --content-base #{PACKS_PATH} #{ARGV.join(" ")}"


--content-base #{PACKS_PATH}

Let's read that from the package.json rather than having the shell script read it.

justin808 · 2017-03-05T08:59:38Z

lib/install/bin/webpack.tt


 Dir.chdir(APP_PATH) do
-  exec "#{SET_NODE_PATH} #{WEBPACK_BIN} --config #{WEBPACK_CONFIG} #{ARGV.join(" ")}"
+  exec "NODE_PATH=#{NODE_MODULES_PATH} #{WEBPACK_BIN} --config #{WEBPACK_CONFIG}" \


see comments from prior file

justin808 · 2017-03-05T09:06:35Z

lib/install/templates/static.rb

+run "./bin/yarn add extract-text-webpack-plugin node-sass file-loader " \
+    "sass-loader css-loader style-loader "
+
+package_json = Webpacker::PackageJson.new().config


what if the user already modified package json?

Maybe webpacker should merge? not replace?

justin808 · 2017-03-05T09:12:53Z

lib/webpacker/manifest.rb

+    end
+  end
+
+  def initialize(path)


for a true singleton, you can make the constructor, etc. private

justin808 · 2017-03-05T09:14:11Z

lib/webpacker/railtie.rb

-                      app.config.x.webpacker[:packs_dist_dir],
-                      'digests.json')
-
-    Webpacker::Digests.load(app.config.x.webpacker[:digests_path])


Big 👍 on removing this code!

justin808 · 2017-03-05T09:16:02Z

lib/webpacker/source.rb

  end

  def path
-    if config[:dev_server_host].present?
-      "#{config[:dev_server_host]}/#{filename}"
+    if Rails.env.development? && config.dev_server[:enabled]


Be sure to allow overriding the dev_sever[:enabled] option by env value, as this is something that is NOT the application config, but really a runtime option!

justin808 · 2017-03-05T09:17:09Z

lib/webpacker/source.rb

    else
-      Webpacker::Digests.lookup(filename)
+      Webpacker::Manifest.load(digests_path)


We should only be reloading this during development mode. RAILS_ENV == 'production' ==> no need to reload.

… plugin Add file and style loaders Use the env options pass and configure file and style loader Setup dev server using env supplied Add node-sass Update production config to use plugins in production mode Refactor compile task as we now use manifest plugin Remove extra whitespace Refactor javascript pack tag and add a new stylesheet pack tag Remove extra logic from source lookup class Use same public path used in shared config Add some checks to make sure valid host and port is passed to dev server Change failure message Remove unwanted env variables Re-order variables Add back NODE_ENV Add the failure check block Just use manifest to lookup files in all environments Remove digesting option Remove digestion option Fix typo Remove reference to digesting in readme Fix error message Remove extra whitespace Remove loader option plugin Fix linter complaints Add example for static assets in readme Remove bash Use file.basename Cleanup webpack configs Fix typo Use env correctly Use hash in production Don't remove .js from babel, instead append jsx to it Export file loader extensions to a variable Redo everything, but first lets add rubocop Copy rubocop.yml from rails repo Change quotes Use package.json and refactor webpack config files Use package.json to load config and get rid of railtie Move templates to separate folder Update webpack binstubs to use package.json as config Lint using rubocop Conditionally require static config Setup a separate install task to setup static assets support Fix typo Add static task Update name and description in package.json Rename config to package_json and update references Add version to separate file and post-install notes Make sure puts write to terminal immediately Rename folder for clarity Update templates to use new path Remove image support from vue installer Update package.json template Update message text Refactor package_json class Add new config class to compile task Block request until digests.json is available Remove .tt Restructure static config location Fix static asset description Preserve whitespace Install static only when needed Use template to insert static support code Cleanup error and warning messages Make sure to check webpacker is installed before running static task Update not found message Update readme with latest changes Refactor classes, variable names and error messages Update post install message Rename static to assets Minor cleanup Fix readme formatting Rename for clarity Wrap it in begin block Minor fixes Wrap everything inside a block Ensure to exit out of the program Add a verify task that gets run before running any dependent task Add the verify task Fix typo Fix missing variable Update postinstall message Fix a typo

dhh · 2017-03-10T12:50:18Z

lib/webpacker/helper.rb

+    unless webpacker_config[:assets]
+      raise StandardError,
+                        "Stylesheet support is not enabled. Install using " \
+                        "webpacker:install:assets"


Just inline this raise entirely. No need to fit in 80 chars.

dhh · 2017-03-10T12:51:20Z

lib/webpacker/package_json.rb

@@ -0,0 +1,58 @@
+# Loads the package.json file from app root to read the webpacker configuration
+
+class Webpacker::PackageJson


I'd go with Webpacker::Configuration instead. The fact that the config is in package.json is an implementational detail.

Ahh yes, that makes more sense 😄

dhh · 2017-03-10T12:52:14Z

webpacker.gemspec


  s.files         = `git ls-files`.split("\n")
  s.test_files    = `git ls-files -- test/*`.split("\n")
+  s.post_install_message = %{


Don't like the use of post install message. Let's move all docs to README.

Alrighty! will remove

dhh · 2017-03-10T12:53:16Z

lib/install/bin/webpack-dev-server.tt

-WEBPACK_CONFIG = "#{ESCAPED_APP_PATH}/config/webpack/#{NODE_ENV}.js"
+begin
+  package_json = File.read(File.join(APP_PATH, 'package.json'))
+  config = JSON.parse(package_json)


No need to have package_json as it's own var. Just inline it like JSON.parse(File.read(File.join(APP_PATH, 'package.json'))).

dhh · 2017-03-10T12:54:25Z

lib/install/bin/webpack-dev-server.tt


-# Warn the user if the configuration is not set
-RAILS_ENV_CONFIG = File.join("config", "environments", "#{RAILS_ENV}.rb")
+  NODE_MODULES_PATH = "#{File.join(ESCAPED_APP_PATH, config['webpacker']['nodeModulesPath'])}"


Don't need to what this in a string. NODE_MODULES_PATH = File.join(ESCAPED_APP_PATH, config['webpacker']['nodeModulesPath']) is good.

Also, do need ESCAPED_APP_PATH and APP_PATH. Just escape APP_PATH.

dhh · 2017-03-10T12:56:07Z

lib/install/bin/webpack-dev-server.tt

-  puts "Warning: if you want to use webpack-dev-server, you need to tell Webpacker to serve asset packs from it. Please set config.x.webpacker[:dev_server_host] in #{RAILS_ENV_CONFIG}.\n\n"
+  if config['devServer'] && !config['devServer']['enabled']
+    puts "Error: If you want to use webpack-dev-server, you will need to enable " \
+        "webpack-dev-server from your package.json { devServer: { enabled: true } }"


This seems weird to me. Why not just assume its enabled if the configuration for it is there?

dhh · 2017-03-10T12:56:47Z

lib/install/bin/webpack-dev-server.tt

+  WEBPACK_CONFIG_PATH = "#{File.join(ESCAPED_APP_PATH, config['webpacker']['configPath'])}"
+  PACKS_PATH = "#{File.join(ESCAPED_APP_PATH, config['webpacker']['distPath'])}"
+  WEBPACK_BIN  = "#{NODE_MODULES_PATH}/.bin/webpack-dev-server"
+  WEBPACK_CONFIG = "#{WEBPACK_CONFIG_PATH}/development.hot.js"


Given all the known issues with hot reloading, maybe it shouldn't be the default.

Perhaps, rename it to development.server.js? We never had hot reloading from start as it requires more setup and got packages for different frameworks. It's just for dev server, that supports live reloading (not hot).

dhh · 2017-03-10T12:57:13Z

lib/install/bin/webpack.tt

+  config = JSON.parse(package_json)
+
+  NODE_MODULES_PATH = "#{File.join(ESCAPED_APP_PATH, config['webpacker']['nodeModulesPath'])}"
+  WEBPACK_CONFIG_PATH = "#{File.join(ESCAPED_APP_PATH, config['webpacker']['configPath'])}"


Same style comments as per the dev server.

dhh · 2017-03-10T12:57:45Z

lib/install/config/assets.js

+        })
+      },
+      {
+        test: /\.(jpeg|png|gif|svg|eot|svg|ttf|woff|woff2)$/i,


What provisions are we supplying to actually reference these assets? I only saw stylesheet getting a pack tag.

No, we don't have any for now. The assets here will be mostly used within javascripts and css as that's where the templates are. An example with react would be,

import React from 'react' import ReactDOM from 'react-dom' import clockIcon from '../counter/images/clock.png'; import './hello-react.sass' const Hello = props => ( <div className="hello-react"> <img src={clockIcon} alt="clock" /> <p>Hello {props.name}!</p> </div> ) Hello.defaultProps = { name: 'David' } Hello.propTypes = { name: React.PropTypes.string } document.addEventListener('DOMContentLoaded', () => { ReactDOM.render( <Hello name="React" />, document.getElementById('react-app'), ) })

@font-face font-family: 'Lacuna' src: url('../fonts/lacuna-webfont.eot') src: url('../fonts/lacuna-webfont.eot?#iefix') format('embedded-opentype'), url('../fonts/lacuna-webfont.woff') format('woff'), url('../fonts/lacuna-webfont.ttf') format('truetype'), url('../fonts/lacuna-webfont.svg#Darkenstone') format('svg') font-weight: normal font-style: normal #css-bg-image width: 48px height: 48px background-image: url('../images/clock.png') background-repeat: no-repeat

Perhaps, we can add an asset_pack_tag later?

dhh · 2017-03-10T12:58:09Z

lib/install/config/webpack/development.hot.js

+    publicPath: devServer.enabled ?
+      `http://${devServer.host}:${devServer.port}/` : `/${sharedConfig.distDir}/`
+  }
+})


Don't see any hot reloading configured here?

Yes, same settings as before, just configurable. We can turn it on, but it will require more setup. This is just for configuring the dev server. Perhaps rename the file to development.server.js?

dhh · 2017-03-10T12:59:38Z

lib/install/config/webpack/shared.js

 const extname = require('path-complete-extname')
+const { webpacker } = require('../../package.json')


I'd do { config } instead and then just reference things directly off that object rather than assigning a bunch of consts.

Oh yes, totally forgot 👍 Will do

dhh · 2017-03-10T13:01:51Z

lib/install/templates/assets.rb

+
+insert_into_file "config/webpack/development.js",
+                  assets_webpack_config,
+                  after: "const sharedConfig = require('./shared.js')\n"


Don't like this indention style. If that's a Rubocop setting, let's change it. Should look like this:

insert_into_file "config/webpack/development.js", assets_webpack_config, after: "const sharedConfig = require('./shared.js')\n"

dhh · 2017-03-10T13:02:06Z

lib/install/templates/install.rb

@@ -0,0 +1,20 @@
+# Setup webpacker
+
+directory "#{File.expand_path("..", __dir__)}/node", "./"


No need for the wrapping "".

Ah, the first one seems like it should use a path join instead.

dhh · 2017-03-10T13:02:59Z

lib/tasks/installers/angular.rake


-      if config.include?('ts-loader')
+      if config.include?("ts-loader")


Style: Use '' for keys etc without interpolation. Use "" for sentences, prose, or for interpolation.

dhh · 2017-03-10T13:04:21Z

lib/webpacker/helper.rb

+
+    filename = File.basename(name, ".css")
+    filename += ".css"
+    stylesheet_link_tag(Webpacker::Source.new(filename).path, **options)


stylesheet_link_tag(Webpacker::Source.new("#{File.basename(name, '.css')}.css").path, **options)

dhh · 2017-03-10T13:05:05Z

lib/webpacker/manifest.rb

+# files, # "/packs/calendar-1016838bab065ae1e314.js" and
+# "/packs/calendar-1016838bab065ae1e314.css" for long-term caching
+
+class Webpacker::Manifest


Why the renaming from Digests?

Previously, it was used for looking up digests on production, but now this is used in all environments to lookup assets using manifest file, not just for digesting. What do you think?

dhh · 2017-03-10T13:05:16Z

lib/webpacker/manifest.rb

+  end
+
+  private
+


✂️ newline.

dhh · 2017-03-10T13:06:37Z

lib/webpacker/package_json.rb

@@ -0,0 +1,58 @@
+# Loads the package.json file from app root to read the webpacker configuration
+
+class Webpacker::PackageJson


This whole thing is too cumbersome. I can see how it's based off configuration, but it feels very boilerplatey.

Agree, I felt too, but later realised that's how most configuration files looks. Anything you think that can make this look simpler?

ytbryan

haha you are fast. is 2017 the year to introduce emoji into rails gems :D

gauravtiwari · 2017-03-22T06:43:55Z

@ytbryan Yeah, Rails got Webpacker 😉

gauravtiwari · 2017-03-23T08:05:44Z

@dhh Added separate loaders support in this commit - d7278d2

I felt this is one last major thing missing. No more overwriting shared.js as it now has very little configuration. Also, refactored integration installers (react etc.) to use thor, which makes it less brittle as thor is really good at it 😄

Not sure if you are okay with extensions? I have added them all by default.

@ytbryan I saw that you added an alias for vue es6 module. For now, I have removed it and referenced it directly from the entry file to isolate shared.js as much as possible.

dhh · 2017-03-23T11:16:47Z

Looking good. Ready to merge?

gauravtiwari · 2017-03-23T11:17:28Z

@dhh Yepp 👍

samandmoore · 2017-03-23T12:39:30Z

This is so exciting! Thanks so much @gauravtiwari for getting this shipped ❤️

gauravtiwari · 2017-03-23T12:44:06Z

@samandmoore Nope, not on purpose, just forgot. But I guess it's easy enough now to add it 😄

rafaelfranca · 2017-03-28T01:54:27Z

.rubocop.yml

@@ -0,0 +1,124 @@
+AllCops:


Can we use the same as rails/rails using the inherit_from option?

I think we should follow https://github.com/github/rubocop-github and create a gem rubocop-rails or something and use it all over the rails repositories with inherit_gem

inherit_from is sufficient. We don't need a gem if we can download the config from github.

this flag was removed in #153, but without it the error handling block throws a `JSON::ParserError` and dumps the webpack output

Not sure if the whole paragraph in the README should go away, but after rails#153 it's not true that webpacker is _only_meant for JS files.

Ifegwu · 2017-04-10T16:31:18Z

Wonderful work! Any way to add bootstrap with webpacker configuration on rails 5.1?

gauravtiwari · 2017-04-10T16:36:58Z

@Ifegwu Here you go -

./bin/yarn add reactstrap react-addons-transition-group react-addons-css-transition-group

// In your main entry/pack file
import 'bootstrap/dist/css/bootstrap.css';

And then basically follow this guide - https://reactstrap.github.io/

If you have any questions please ask on StackOverflow

Ifegwu · 2017-04-10T16:43:15Z

Thanks for your swift response. Can this method be able to style devise?

mltsy · 2017-07-27T21:35:09Z

I have no issues, just a big thank you for this feature! This is GREAT! 😄 👏 🙏

This was referenced Mar 3, 2017

Webpacker::Digests.lookup issue #151

Closed

Make sure Digest::lookup always pick the correct js file name. #152

Closed

justin808 reviewed Mar 3, 2017

View reviewed changes

justin808 suggested changes Mar 5, 2017

View reviewed changes

justin808 mentioned this pull request Mar 5, 2017

Suggestions on Directory Organization #130

Closed

gauravtiwari added 2 commits March 6, 2017 06:52

Add stylesheet_pack_tag helper doc to readme

769cd06

gauravtiwari mentioned this pull request Mar 10, 2017

Add option to have css/images managed through webpack #109

Closed

dhh reviewed Mar 10, 2017

View reviewed changes

lib/webpacker/manifest.rb Outdated

end

private

Copy link

Member

dhh Mar 10, 2017

Choose a reason for hiding this comment

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

✂️ newline.

dhh reviewed Mar 10, 2017

View reviewed changes

ytbryan reviewed Mar 22, 2017

View reviewed changes

gauravtiwari added 4 commits March 22, 2017 13:17

Make sure to use paths.yml for third-party installers

ec86811

Use paths from configuration

101c4f6

Remove filename

5842c70

Refactor loaders and installers

d7278d2

dhh merged commit a858f8e into rails:master Mar 23, 2017

gauravtiwari deleted the feature/static-assets branch March 23, 2017 11:21

This was referenced Mar 23, 2017

Use relative node_modules path for webpack resolve.modules config #166

Closed

Suggested Refinements on the Purpose and Scope of Webpacker #139

Closed

ytbryan mentioned this pull request Mar 24, 2017

Image loading breaks for vue default pack #183

Closed

This was referenced Mar 26, 2017

Errors in Rails system tests when webpack-dev-server is running #189

Closed

Scope webpack config through env #191

Merged

thomas-mcdonald mentioned this pull request Mar 28, 2017

run ./bin/webpack with --json flag in webpacker:compile #199

Merged

rafaelfranca reviewed Mar 28, 2017

View reviewed changes

dhh pushed a commit that referenced this pull request Mar 28, 2017

run ./bin/webpack with --json flag in webpacker:compile (#199)

c4d912d

this flag was removed in #153, but without it the error handling block throws a `JSON::ParserError` and dumps the webpack output

matthewd mentioned this pull request Apr 2, 2017

Refactor, css-modules, spring, hot module replacement and fixes #219

Closed

claudiob added a commit to claudiob/webpacker that referenced this pull request Apr 10, 2017

webpacker can also manage non-JS assets [ci skip]

106cfb3

Not sure if the whole paragraph in the README should go away, but after rails#153 it's not true that webpacker is _only_meant for JS files.

claudiob mentioned this pull request Apr 10, 2017

webpacker can also manage non-JS assets [ci skip] #249

Closed

gauravtiwari mentioned this pull request Apr 10, 2017

view helper not adding digest string to file name #244

Closed

taboularasa mentioned this pull request May 16, 2017

recommended way to use Angular templateUrl? #394

Closed

		@@ -0,0 +1,58 @@
		# Loads the package.json file from app root to read the webpacker configuration

		class Webpacker::PackageJson

		const extname = require('path-complete-extname')
		const { webpacker } = require('../../package.json')

		@@ -0,0 +1,20 @@
		# Setup webpacker

		directory "#{File.expand_path("..", __dir__)}/node", "./"


		if config.include?('ts-loader')
		if config.include?("ts-loader")

Add static assets support #153

Add static assets support #153

Conversation

gauravtiwari commented Mar 2, 2017 • edited Loading

Major Changes

Minor updates

To try this out locally,

Deploy it to heroku

dleavitt commented Mar 2, 2017

gauravtiwari commented Mar 2, 2017

justin808 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gauravtiwari commented Mar 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

How about 'railsConfig'?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhh Mar 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhh Mar 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gauravtiwari Mar 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ytbryan left a comment

Choose a reason for hiding this comment

gauravtiwari commented Mar 22, 2017

gauravtiwari commented Mar 23, 2017

dhh commented Mar 23, 2017

gauravtiwari commented Mar 23, 2017

samandmoore commented Mar 23, 2017

gauravtiwari commented Mar 23, 2017

gauravtiwari commented Mar 2, 2017 •

edited

Loading

dhh Mar 10, 2017 •

edited

Loading

dhh Mar 10, 2017 •

edited

Loading

gauravtiwari Mar 10, 2017 •

edited

Loading