Updates for v2: Simplified configuration #9
Conversation
|
@kaizencodes @robwise @Conturbo please review. |
| @@ -17,7 +17,7 @@ def hot_loading? | |||
| end | |||
|
|
|||
| def file_path | |||
| Rails.root.join("config", "webpack", "paths.yml") | |||
| Rails.root.join("config", "webpacker_lite.yml") | |||
There was a problem hiding this comment.
Apparently, the same code is used in configuration.rb. Might be worth having it dried.
lib/webpacker_lite/helper.rb
Outdated
| # | ||
| # # In production mode: | ||
| # <%= javascript_pack_tag 'calendar', 'data-turbolinks-track': 'reload' %> # => | ||
| # <script src="/packs/calendar-1016838bab065ae1e314.js" data-turbolinks-track="reload"></script> | ||
| # <script src="/public/webpack/production/calendar-1016838bab065ae1e314.js" data-turbolinks-track="reload"></script> | ||
| def javascript_pack_tag(name, **options) | ||
| javascript_include_tag(WebpackerLite::Manifest.lookup("#{name}#{compute_asset_extname(name, type: :javascript)}"), **options) |
There was a problem hiding this comment.
Hard to read, I'd consider extracting a method or two.
lib/webpacker_lite/helper.rb
Outdated
| asset_type = WebpackerLite::Env.hot_loading? ? :hot : :static | ||
| names = Array(kwargs[asset_type]) | ||
| manifested_names += get_manifests(names) | ||
| return "" if WebpackerLite::Env.hot_loading? && !kwargs[enabled_when_hot_loading].presence |
There was a problem hiding this comment.
It looks like enabled_when_hot_loading might need to be a symbol.
There was a problem hiding this comment.
So, the tests didn't catch it? Might be worth adding a test to cover that.
test/helper_test.rb
Outdated
| end | ||
|
|
||
| def test_stylesheet_pack_tag | ||
| style = %(<link rel="stylesheet" media="screen" href="/webpack/bootstrap-c38deda30895059837cf.css" />) |
There was a problem hiding this comment.
I'd get rid of the variable style and just put the literal on 2nd line. If you want to keep it, I'd call it expected_style or something like that.
test/manifest_test.rb
Outdated
| end | ||
|
|
||
| def test_lookup_success | ||
| asset_file = "bootstrap.js" |
There was a problem hiding this comment.
Usually, a variable in these tests is a literal that the result of the tested method is compared to. In this case, it's the literal of the tested method's argument. Breaks expectations.
justin808
changed the title
|
Hey! big question for @alexfedoseev @robwise @Judahmeek regarding hot reloading and webpacker_lite. Should webpacker_lite be putting in the value of the hot reloading server host:port as part of the rails helper, or depending on the configuration of that from the manifest.json and requiring the setting of the publicPath in both the webpack output and manifest plugin setup. My gut tells me that it’s better to keep the webpack manifest simpler so it’s really the mapping of output names to possibly fingerprinted filenames. |
justin808
changed the title
2.0.0 - 2017-05-23 * Rewrote README.md. * Configuration is simplified and changed to a single file, /config/webpacker_lite.yml. See README.md. * v1 assumed that manifest.json would contain the host name for hot reloading. v2 puts in the host at the Ruby level. * stylesheet_pack_tag API changed. ENV value for HOT_RELOADING == "TRUE" results in the stylesheet_pack_tag not writing anything due to hot reloading requiring inlined JavaScript of * styles and not extracted CSS. * Removed any bits of JavaScript from webpacker_lite. * Added tests
This change is