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.

Sign up for GitHub

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

Introduces support for sprockets 4.y.z releases #311

Closed
wants to merge 5 commits into from
Closed

Introduces support for sprockets 4.y.z releases #311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d9779f8
Loosening the version requirements for sass-rails in order to permit …
jrgriffiniii Nov 1, 2019
4f1c61a
Pinning the sass-rails dependency to 5.y.z releases just for Rails 6.0.0
jrgriffiniii Nov 1, 2019
2d605f6
Inserting sprockets for Rails 6.0.z releases
jrgriffiniii Nov 4, 2019
1b48537
Adding Webpacker as a dependency for Rails 6.0.z builds
jrgriffiniii Nov 4, 2019
754b4eb
Rails 6.0.0 does not support Ruby 2.4.z releases
jrgriffiniii Nov 4, 2019
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
20 changes: 14 additions & 6 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,27 +45,35 @@ jobs:
workflows:
ci:
jobs:
- build:
name: "ruby2-6_rails6-0"
ruby_version: 2.6.5
rails_version: 6.0.0
- build:
name: "ruby2-6_rails5-2"
ruby_version: 2.6.3
ruby_version: 2.6.5
rails_version: 5.2.3
- build:
name: "ruby2-6_rails5-1"
ruby_version: 2.6.3
ruby_version: 2.6.5
rails_version: 5.1.7
- build:
name: "ruby2-5_rails6-0"
ruby_version: 2.5.7
rails_version: 6.0.0
- build:
name: "ruby2-5_rails5-2"
ruby_version: 2.5.5
ruby_version: 2.5.7
rails_version: 5.2.3
- build:
name: "ruby2-5_rails5-1"
ruby_version: 2.5.5
ruby_version: 2.5.7
rails_version: 5.1.7
- build:
name: "ruby2-4_rails5-2"
ruby_version: 2.4.6
ruby_version: 2.4.9
rails_version: 5.2.3
- build:
name: "ruby2-4_rails5-1"
ruby_version: 2.4.6
ruby_version: 2.4.9
rails_version: 5.1.7
9 changes: 8 additions & 1 deletion Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,21 @@ else
end

case ENV['RAILS_VERSION']
when '6.0.0' # This will only be necessary until release 6.0.1 is published
gem 'sass-rails', '~> 5'
gem 'webpacker' # This is necessary or the generator will break
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This shouldn't be here, but I cannot disable engine_cart from inserting webpacker as a dependency.

when /^5\./
gem 'capybara', '~> 2.18.0'
when /^4\.2/
gem 'sass-rails', '~> 5'
when /^4\./
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This needs to be removed, Core Components are no longer required to support Rails 4.y.z releases.

gem 'coffee-rails', '~> 4.1.0'
gem 'json', '~> 1.8'
gem 'railties', '~> 4.2'
gem 'responders', '~> 2.0'
gem 'sass-rails', '>= 5.0'
else
gem 'sass-rails'
gem 'webpacker' # This is necessary or the generator will break
end
end
# END ENGINE_CART BLOCK
Expand Down
4 changes: 2 additions & 2 deletions browse-everything.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ Gem::Specification.new do |spec|
spec.add_dependency 'google_drive', '~> 2.1'
spec.add_dependency 'googleauth', '0.6.6'
spec.add_dependency 'puma', '~> 3.11'
spec.add_dependency 'rails', '>= 4.2', '< 6.0'
spec.add_dependency 'rails', '>= 4.2'
spec.add_dependency 'ruby-box'
spec.add_dependency 'signet', '~> 0.8'
spec.add_dependency 'sprockets', '~> 3.7'
spec.add_dependency 'sprockets'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sprockets becomes a problem with engine_cart unless --skip-sprockets is passed to the Rails generator. This needs to be resolved in order to remove sprockets as a dependency of the Gem (at least, for Rails 6.y.z releases).

spec.add_dependency 'thor', '~> 0.19'
spec.add_dependency 'typhoeus'

Expand Down
3 changes: 3 additions & 0 deletions spec/test_app_templates/app/assets/javascripts/application.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
//= require_tree .
//= require jquery
//= require browse_everything
27 changes: 22 additions & 5 deletions spec/test_app_templates/lib/generators/test_app_generator.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,28 @@ def inject_css
end

def inject_javascript
insert_into_file 'app/assets/javascripts/application.js', after: '//= require_tree .' do
%(
//= require jquery
//= require browse_everything
)
if Rails.version =~ /^6\./
copy_file 'app/assets/javascripts/application.js', 'app/assets/javascripts/application.js'

# Does this means that Webpacker becomes a hard-dependency?
Copy link
Contributor

Choose a reason for hiding this comment

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

Nope, this has nothing to do with webpacker. In fact, it's the opposite -- what this line of code is doing is making sure that Sprockets will still handle Javascript, even if you are using Sprockets 4.

Since:

  • Sprockets 4 (unlike sprockets 3) will use this app/assets/config/mannifest.js file
  • Rails 6 (unlike Rails 5) will not include the link_directory ../javascripts .js line in the generated app/assets/config/manifest.js file, since Rails 6 thinks you shouldn't use Sprockets for JS.

So what you're doing is making it so Sprockets will still do JS in Rails 6.0.1 with Sprockets 4 -- nothing to do with webpacker at all.

So at least, you probably want to remove/change this comment.

But also, since this is a generator -- you probably want to document where (README?) that the browse-everything installer under Rails 6 is going to re-enable Sprockets handling of JS, and add a line to your application.html.erb layout that generates a <script> tag for a sprockets-compiled application.js in addition to the existing webpacker-compiled one.

Which is not ideal -- but is better than browse-everything not working at all, it's what we've got for Rails 6 right now.

insert_into_file 'app/assets/config/manifest.js', after: '//= link_directory ../stylesheets .css' do
%(
//= link_directory ../javascripts .js
)
end

insert_into_file 'app/views/layouts/application.html.erb', after: "<%= javascript_pack_tag 'application', 'data-turbolinks-track': 'reload' %>" do
%(
<%= javascript_include_tag 'application' %>
)
end
else
insert_into_file 'app/assets/javascripts/application.js', after: '//= require_tree .' do
%(
//= require jquery
//= require browse_everything
)
end
end
end

Expand Down