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

Support singleton webpacker (3 and above) #777

Merged
merged 50 commits into from
Sep 18, 2017
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
392ec00
Support singleton webpacker (3 and above)
BookOfGreg Aug 31, 2017
4531b1c
Support Specific versions of webpacker in appraisal
BookOfGreg Aug 31, 2017
14865d8
Update appraisal
BookOfGreg Aug 31, 2017
80c03d8
Simplify
BookOfGreg Aug 31, 2017
8c7dcd8
Pin to 3 or greater
BookOfGreg Aug 31, 2017
cd74273
Compatibility for Webpacker new
BookOfGreg Aug 31, 2017
2b7cf68
Reference correct constant
BookOfGreg Sep 1, 2017
d1e665e
Fixes #778
BookOfGreg Sep 5, 2017
ea8810e
Handle manifest load
BookOfGreg Sep 5, 2017
5a78e5d
WIP, trying to get webpacker3 config with webpacker1 and 2
BookOfGreg Sep 5, 2017
b960b9a
Extract out data
BookOfGreg Sep 5, 2017
cc8528f
Ruby 2.1 doesnt support twiddledocs
BookOfGreg Sep 5, 2017
54f95b7
Take on the configuration comment, also reduce the react regex as the…
BookOfGreg Sep 8, 2017
ed8a891
Split out dummy apps for the asset pipelines.
BookOfGreg Sep 8, 2017
488a840
Split out Webpacker dummy and sprockets dummy, then change to that di…
BookOfGreg Sep 8, 2017
2c8ec5e
ignore ruby 2.1 with Rails 5
BookOfGreg Sep 8, 2017
29c1d02
Add new dummy webpacker3 app
BookOfGreg Sep 8, 2017
ca48da9
Typo in appraisal gemfile name
BookOfGreg Sep 8, 2017
41b633f
Typo in appraisal gemfile name
BookOfGreg Sep 8, 2017
a5f3da5
Pin webpacker 1.1
BookOfGreg Sep 8, 2017
9b7af66
Set directory based on gem version
BookOfGreg Sep 8, 2017
2492053
Define methods based on which webpacker version is loaded to make con…
BookOfGreg Sep 11, 2017
f33976a
Update appraisals for each version of webpacker
BookOfGreg Sep 11, 2017
1b210b0
Each one seems to need turbolinks
BookOfGreg Sep 11, 2017
72285ce
Shouldnt need turbolinks
BookOfGreg Sep 11, 2017
8c88a3b
Turns out _no_sprockets_ name is actually executable code...
BookOfGreg Sep 11, 2017
4fad8b3
Reinitialize webpacker with default config except for the following d…
BookOfGreg Sep 11, 2017
c587945
Add new Webpacker 2 folder
BookOfGreg Sep 11, 2017
b6dc2aa
Regenerate webpacker configs for webpacker 2
BookOfGreg Sep 11, 2017
7b1b5ad
Tweaking configs and requires to sensible version
BookOfGreg Sep 11, 2017
6324185
Use local node_module
BookOfGreg Sep 11, 2017
77536cd
Dont test Rails 5 on ruby 2.1
BookOfGreg Sep 11, 2017
253ccde
Make it use localhost rather than 0.0.0.0, also React 15.4 so I do no…
BookOfGreg Sep 11, 2017
2805d9d
Add Webpacker 3 test app
BookOfGreg Sep 11, 2017
9390b09
Webpack 3 settings
BookOfGreg Sep 11, 2017
4aa297f
Tweak dev-server detector
BookOfGreg Sep 12, 2017
f15b255
Use Webpackers native dev_server detection
BookOfGreg Sep 12, 2017
c61f74d
properly scope config access
BookOfGreg Sep 12, 2017
0c0b9bc
Remove require pry references
BookOfGreg Sep 12, 2017
e3efcb8
Introduce rubocop to project
BookOfGreg Sep 15, 2017
0f75fe7
rubocop --only Layout/SpaceInsideHashLiteralBraces -a
BookOfGreg Sep 15, 2017
07afcb7
rubocop --only Style/StringLiterals -a
BookOfGreg Sep 15, 2017
384a406
Test scanned the file for specific quotes.
BookOfGreg Sep 18, 2017
579367f
rubocop --only Layout/SpaceInsideBlockBraces -a
BookOfGreg Sep 18, 2017
940f5dd
rubocop --only Style/NegatedIf -a
BookOfGreg Sep 18, 2017
9437671
rubocop --only Style/TrailingCommaInLiteral -a
BookOfGreg Sep 18, 2017
caf0827
rubocop --only Layout/SpaceInsideBrackets -a
BookOfGreg Sep 18, 2017
f2330d9
rubocop --only Layout/SpaceBeforeComment -a
BookOfGreg Sep 18, 2017
e5bbe14
rubocop --only Style/ParallelAssignment -a
BookOfGreg Sep 18, 2017
83fbdaf
rubocop --only Layout/EmptyLines -a
BookOfGreg Sep 18, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ rvm:
gemfile:
# These have webpacker:
- gemfiles/rails_4.2_sprockets_4.gemfile
- gemfiles/rails_5_no_sprockets_webpacker.gemfile
- gemfiles/rails_5_no_sprockets_webpacker_1.gemfile
- gemfiles/rails_5_no_sprockets_webpacker_3.gemfile
# These don't have webpacker:
- gemfiles/rails_3.2.gemfile
- gemfiles/rails_4.0.5.gemfile
Expand Down
15 changes: 11 additions & 4 deletions Appraisals
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,22 @@ appraise "rails-4.2-sprockets_4" do
gem 'rails', '~> 4.2.1'
gem "sprockets", "~> 4.0.x"
gem "turbolinks", "~> 2.5.0"
gem "webpacker", github: "rails/webpacker"
# This ExecJS backend provides stateful context
# which the default nodejs backend does not
gem "mini_racer"
end

appraise "rails-5_no_sprockets_webpacker" do
appraise "rails-5_no_sprockets_webpacker_1" do
gem 'rails', '~> 5.0.0'
gem "webpacker", github: "rails/webpacker"
gem "webpacker", '~> 1.0'
# This ExecJS backend provides stateful context
# which the default nodejs backend does not
gem "therubyracer"
end

appraise "rails-5_no_sprockets_webpacker_3" do
gem 'rails', '~> 5.0.0'
gem 'webpacker', '>= 3.0 '
# This ExecJS backend provides stateful context
# which the default nodejs backend does not
gem "therubyracer"
Expand All @@ -66,7 +73,7 @@ appraise "rails-5-no_sprockets" do
end

appraise "rails-5.1-sprockets_4" do
gem "rails", "5.1.0.rc1"
gem "rails", "~> 5.1"
gem "sprockets", "~> 4.0.x"
gem "turbolinks", "~> 5.0.0"
end
1 change: 0 additions & 1 deletion gemfiles/rails_4.2_sprockets_4.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ source "http://rubygems.org"
gem "turbolinks", "~> 2.5.0"
gem "rails", "~> 4.2.1"
gem "sprockets", "~> 4.0.x"
gem "webpacker", :github => "rails/webpacker"
gem "mini_racer"

gemspec :path => "../"
2 changes: 1 addition & 1 deletion gemfiles/rails_5.1_sprockets_4.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
source "http://rubygems.org"

gem "turbolinks", "~> 5.0.0"
gem "rails", "5.1.0.rc1"
gem "rails", "~> 5.1"
gem "sprockets", "~> 4.0.x"

gemspec :path => "../"
1 change: 0 additions & 1 deletion gemfiles/rails_5_no_sprockets.gemfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# This file was generated by Appraisal
# This shouldn't have turbolinks or sprockets

source "http://rubygems.org"

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
# This file was generated by Appraisal
# This file shouldn't have turbolinks or sprockets in it

source "http://rubygems.org"

gem "rails", "~> 5.0.0"
gem "webpacker", :github => "rails/webpacker"
gem "webpacker", "~> 1.0"
gem "therubyracer"

gemspec :path => "../"
9 changes: 9 additions & 0 deletions gemfiles/rails_5_no_sprockets_webpacker_3.gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# This file was generated by Appraisal

source "http://rubygems.org"

gem "rails", "~> 5.0.0"
gem "webpacker", ">= 3.0 "
gem "therubyracer"

gemspec :path => "../"
6 changes: 5 additions & 1 deletion lib/generators/react/component_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def create_component_file
if webpacker?
new_file_name = file_name.camelize
extension = options[:coffee] ? "coffee" : "js"
target_dir = Webpacker::Configuration.source_path
target_dir = webpack_configuration.source_path
.join("components")
.relative_path_from(::Rails.root)
.to_s
Expand All @@ -119,6 +119,10 @@ def create_component_file

private

def webpack_configuration
Webpacker.respond_to?(:config) ? Webpacker.config : Webpacker::Configuration
end

def component_name
file_name.camelize
end
Expand Down
13 changes: 11 additions & 2 deletions lib/generators/react/install_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ def webpacker?

def javascript_dir
if webpacker?
Webpacker::Configuration.source_path
.join(Webpacker::Configuration.entry_path)
webpack_source_path
.relative_path_from(::Rails.root)
.to_s
else
Expand Down Expand Up @@ -110,6 +109,16 @@ def setup_react_webpacker
create_file(manifest, WEBPACKER_SETUP_UJS)
end
end

private

def webpack_source_path
if Webpacker.respond_to?(:config)
Webpacker.config.source_entry_path # Webpacker >3

Choose a reason for hiding this comment

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

it works for me

Webpacker.config.source_path.join(Webpacker.config.source_entry_path)

Choose a reason for hiding this comment

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

It works..... but is it correct?

Choose a reason for hiding this comment

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

I think this is correct as-is: Webpacker.config.source_entry_path

Choose a reason for hiding this comment

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

Webpacker.config.source_entry_path is the correct way to call this, as this is defined in webpacker.

else
Webpacker::Configuration.source_path.join(Webpacker::Configuration.entry_path) # Webpacker <3
end
end
end
end
end
16 changes: 14 additions & 2 deletions lib/react/server_rendering/webpacker_manifest_container.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class WebpackerManifestContainer

def find_asset(logical_path)
# raises if not found
asset_path = Webpacker::Manifest.lookup(logical_path).to_s
asset_path = manifest.lookup(logical_path).to_s
if asset_path.start_with?("http")
# Get a file from the webpack-dev-server
dev_server_asset = open(asset_path).read
Expand All @@ -21,11 +21,23 @@ def find_asset(logical_path)
dev_server_asset
else
# Read the already-compiled pack:
full_path = Webpacker::Manifest.lookup_path(logical_path).to_s
full_path = file_path(logical_path).to_s
File.read(full_path)
end
end

def manifest
Webpacker.respond_to?(:manifest) ? Webpacker.manifest : Webpacker::Manifest
end

def file_path path
manifest.respond_to?(:lookup_path) ? manifest.lookup_path(path) : File.join(config.output_path, manifest.lookup(path).split('/')[2..-1])
end

def config
!!defined?(Webpacker::Configuration) ? Webpacker::Configuration : Webpacker.config

Choose a reason for hiding this comment

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

I think this needs to be

def config
  Webpacker.respond_to?(:config) ? Webpacker.config : Webpacker::Configuration
end

Because even in Webpacker 3, the Webpacker::Configuration constant is defined.

end

def self.compatible?
!!defined?(Webpacker)
end
Expand Down
28 changes: 16 additions & 12 deletions test/dummy/config/webpack/development.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
// Note: You must restart bin/webpack-dev-server for changes to take effect
try {
const environment = require('./environment')

const merge = require('webpack-merge')
const sharedConfig = require('./shared.js')
module.exports = environment.toWebpackConfig()
} catch (e) {
const merge = require('webpack-merge')
const sharedConfig = require('./shared.js')

module.exports = merge(sharedConfig, {
devtool: 'sourcemap',
module.exports = merge(sharedConfig, {
devtool: 'sourcemap',

stats: {
errorDetails: true
},
stats: {
errorDetails: true
},

output: {
pathinfo: true
}
})
output: {
pathinfo: true
}
})
}
3 changes: 3 additions & 0 deletions test/dummy/config/webpack/environment.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const { environment } = require('@rails/webpacker')

module.exports = environment
1 change: 0 additions & 1 deletion test/dummy/config/webpack/paths.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ development:

test:
<<: *default
manifest: manifest-test.json

production:
<<: *default
33 changes: 16 additions & 17 deletions test/dummy/config/webpack/production.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
// Note: You must restart bin/webpack-dev-server for changes to take effect
try {
const environment = require('./environment')

/* eslint global-require: 0 */
module.exports = environment.toWebpackConfig()
} catch (e) {
const merge = require('webpack-merge')
const sharedConfig = require('./shared.js')

const webpack = require('webpack')
const merge = require('webpack-merge')
const CompressionPlugin = require('compression-webpack-plugin')
const sharedConfig = require('./shared.js')
module.exports = merge(sharedConfig, {
devtool: 'sourcemap',

module.exports = merge(sharedConfig, {
output: { filename: '[name]-[chunkhash].js' },
stats: {
errorDetails: true
},

plugins: [
new webpack.optimize.UglifyJsPlugin(),
new CompressionPlugin({
asset: '[path].gz[query]',
algorithm: 'gzip',
test: /\.(js|css|html|json|ico|svg|eot|otf|ttf)$/
})
]
})
output: {
pathinfo: true
}
})
}
22 changes: 18 additions & 4 deletions test/dummy/config/webpack/test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,20 @@
// Note: You must restart bin/webpack-dev-server for changes to take effect
try {
const environment = require('./environment')

const merge = require('webpack-merge')
const sharedConfig = require('./shared.js')
module.exports = environment.toWebpackConfig()
} catch (e) {
const merge = require('webpack-merge')
const sharedConfig = require('./shared.js')

module.exports = merge(sharedConfig, {})
module.exports = merge(sharedConfig, {
devtool: 'sourcemap',

stats: {
errorDetails: true
},

output: {
pathinfo: true
}
})
}
56 changes: 56 additions & 0 deletions test/dummy/config/webpacker.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# Note: You must restart bin/webpack-dev-server for changes to take effect

default: &default # ~ = Rails.root
source_path: app/javascript # ~/:source
source_entry_path: packs # ~/:source/:entry
public_output_path: packs # ~/public/:output
cache_path: tmp/cache/webpacker

# Additional paths webpack should lookup modules
# ['app/assets', 'engine/foo/app/assets']
resolved_paths: []

# Reload manifest.json on all requests so we reload latest compiled packs
cache_manifest: false

extensions:
- .coffee
- .erb
- .js
- .jsx
- .ts
- .vue
- .sass
- .scss
- .css
- .png
- .svg
- .gif
- .jpeg
- .jpg

development:
<<: *default
compile: true

dev_server:
host: localhost
port: 3035
hmr: false
https: false

test:
<<: *default
compile: true

# Compile test packs to a separate directory
public_output_path: packs-test

production:
<<: *default

# Production demands on precompilation of packs prior to booting for performance.
compile: false

# Cache manifest.json for performance
cache_manifest: true
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
require "test_helper"
require "open-uri"
require 'pry'

WebpackerHelpers.when_webpacker_available do
class WebpackerManifestContainerTest < ActiveSupport::TestCase
setup do
WebpackerHelpers.clear_webpacker_packs
end

test "it loads JS from the webpacker container" do
def test_it_loads_JS_from_the_webpacker_container
WebpackerHelpers.compile
container = React::ServerRendering::WebpackerManifestContainer.new
js_file = container.find_asset("application.js")
Expand Down
14 changes: 7 additions & 7 deletions test/server_rendered_html_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,15 @@ def wait_to_ensure_asset_pipeline_detects_changes

if WebpackerHelpers.available?
new_file_path = '../dummy/app/javascript/components/NewList.js'
new_file_contents = <<-JS
var React = require("react")
module.exports = function() { return <span>"New List"</span> }
JS
new_file_contents = <<-HEREDOC
var React = require("react")
module.exports = function() { return <span>"New List"</span> }
HEREDOC
else
new_file_path = '../dummy/app/assets/javascripts/components/ZZ_NewComponent.js.jsx'
new_file_contents = <<-JS
var NewList = function() { return <span>"New List"</span> }
JS
new_file_contents = <<-HEREDOC
var NewList = function() { return <span>"New List"</span> }
Copy link

Choose a reason for hiding this comment

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

Seems like you might've messed up the indentation here

Choose a reason for hiding this comment

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

The <<- syntax allows the closing HEREDOC to be indented but it won't strip space from the content so this works just fine.

HEREDOC
end

new_file_path = File.expand_path(new_file_path, __FILE__)
Expand Down
Loading