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 Importmap and revise Webpack support #3488

Merged
merged 6 commits into from
Jun 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 7 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
gemfile: [gemfiles/rails_7.0.gemfile]
orm: [active_record]
adapter: [sqlite3]
asset: [webpacker]
asset: [webpack]
include:
- ruby: 2.6
gemfile: gemfiles/rails_6.0.gemfile
Expand All @@ -33,7 +33,7 @@ jobs:
gemfile: gemfiles/rails_7.0.gemfile
orm: active_record
adapter: mysql2
asset: sprockets
asset: importmap
- ruby: "3.0"
gemfile: gemfiles/rails_7.0.gemfile
orm: active_record
Expand Down Expand Up @@ -107,13 +107,18 @@ jobs:
- name: Setup application
env:
BUNDLE_GEMFILE: ../../${{ matrix.gemfile }}
CI_ASSET: ${{ matrix.asset }}
CI_DB_ADAPTER: ${{ matrix.adapter }}
RAILS_ENV: test
run: |
yarn install
cd spec/dummy_app
bundle exec rake rails_admin:prepare_ci_env db:create db:migrate
yarn install
case "$CI_ASSET" in
"webpack" ) yarn build && yarn build:css ;;
"importmap" ) yarn build:css ;;
esac
cd ../../
- name: Run tests
run: bundle exec rspec
Expand Down
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,6 @@ spec/dummy_app/db/*.sqlite3-journal
spec/dummy_app/db/schema.rb
spec/dummy_app/log/*.log
spec/dummy_app/public/uploads
spec/dummy_app/Gemfile.lock
spec/dummy_app/Gemfile*.lock
tmp/**/*
nbproject
1 change: 1 addition & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
coverage
lib/generators/rails_admin/templates
spec/dummy_app/app/assets/builds
spec/dummy_app/public
spec/dummy_app/tmp
spec/support/jquery.simulate.drag-sortable.js
Expand Down
1 change: 1 addition & 0 deletions Appraisals
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ end

appraise 'rails-7.0' do
gem 'rails', '~> 7.0.0'
gem 'importmap-rails', require: false
gem 'sassc-rails', '~> 2.1'
gem 'devise', '~> 4.8'

Expand Down
9 changes: 9 additions & 0 deletions app/views/layouts/rails_admin/_head.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@
<% when :webpack %>
<%= stylesheet_link_tag "rails_admin.css", media: :all, data: {'turbo-track': 'reload'} %>
<%= javascript_include_tag "rails_admin.js", async: true, data: {'turbo-track': 'reload'} %>
<% when :importmap %>
<%= stylesheet_link_tag "rails_admin.css", media: :all, data: {'turbo-track': 'reload'} %>
<%= javascript_inline_importmap_tag(RailsAdmin::Engine.importmap.to_json(resolver: self)) %>
<%= javascript_importmap_module_preload_tags(RailsAdmin::Engine.importmap) %>
<%= javascript_importmap_shim_nonce_configuration_tag %>
<%= javascript_importmap_shim_tag %>
<%= # Preload jQuery and make it global, unless jQuery UI fails to initialize
tag.script "import jQuery from 'jquery'; window.jQuery = jQuery;".html_safe, type: "module" %>
<%= javascript_import_module_tag 'rails_admin' %>
<% else
raise "Unknown asset_source: #{RailsAdmin::config.asset_source}"
end %>
1 change: 1 addition & 0 deletions gemfiles/rails_7.0.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ gem "devise", "~> 4.8"
gem "rails", "~> 7.0.0"
gem "webpacker", require: false
gem "webrick", "~> 1.7"
gem "importmap-rails", require: false
gem "sassc-rails", "~> 2.1"

group :active_record do
Expand Down
28 changes: 28 additions & 0 deletions lib/generators/rails_admin/importmap_formatter.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# frozen_string_literal: true

require 'importmap/packager'

module RailsAdmin
class ImportmapFormatter
attr_reader :packager

def initialize(path = 'confing/importmap.rails_admin.rb')
@packager = Importmap::Packager.new(path)
end

def format
imports = packager.import("rails_admin@#{RailsAdmin::Version.js}")

# Use ESM compatible version to work around https://github.com/cljsjs/packages/issues/1579
imports['@popperjs/core'].gsub!('lib/index.js', 'dist/esm/popper.js')

# Tidy up jQuery UI dependencies
jquery_uis = imports.keys.filter { |key, _| key =~ /jquery-ui/ }
imports['jquery-ui/'] = imports[jquery_uis.first].gsub(%r{(@[^/@]+)/[^@]+$}, '\1/')
imports.reject! { |key, _| jquery_uis.include? key }

pins = ['pin "rails_admin", preload: true', packager.pin_for('rails_admin/src/rails_admin/base', imports.delete('rails_admin'))]
(pins + imports.map { |package, url| packager.pin_for(package, url) }).join("\n")
end
end
end
75 changes: 66 additions & 9 deletions lib/generators/rails_admin/install_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class InstallGenerator < Rails::Generators::Base
include Generators::Utils::InstanceMethods

argument :_namespace, type: :string, required: false, desc: 'RailsAdmin url namespace'
class_option :asset, type: :string, required: false, default: nil, desc: 'Asset delivery method [options: webpacker, sprockets]'
class_option :asset, type: :string, required: false, default: nil, desc: 'Asset delivery method [options: webpacker, webpack, sprockets, importmap]'
desc 'RailsAdmin installation generator'

def install
Expand All @@ -29,10 +29,14 @@ def install
case asset
when 'webpack'
configure_for_webpack
when 'importmap'
configure_for_importmap
when 'webpacker'
configure_for_webpacker5
when 'sprockets'
configure_for_sprockets
else
raise "Unknown asset source: #{asset}"
end
end

Expand All @@ -43,6 +47,10 @@ def asset

if defined?(Webpacker)
'webpacker'
elsif Rails.root.join('webpack.config.js').exist?
'webpack'
elsif Rails.root.join('config/importmap.rb').exist?
'importmap'
else
'sprockets'
end
Expand All @@ -54,17 +62,66 @@ def configure_for_sprockets

def configure_for_webpacker5
run "yarn add rails_admin@#{RailsAdmin::Version.js}"
@scss_relative_dir = '../stylesheets/'
template 'rails_admin.js.erb', 'app/javascript/packs/rails_admin.js'
template 'rails_admin.scss', 'app/javascript/stylesheets/rails_admin.scss'
template 'rails_admin.webpacker.js', 'app/javascript/packs/rails_admin.js'
template 'rails_admin.scss.erb', 'app/javascript/stylesheets/rails_admin.scss'
end

def configure_for_webpack
run "yarn add rails_admin@#{RailsAdmin::Version.js} css-loader mini-css-extract-plugin sass sass-loader"
@scss_relative_dir = './'
template 'rails_admin.js.erb', 'app/javascript/rails_admin.js'
template 'rails_admin.scss', 'app/javascript/rails_admin.scss'
template 'webpack.config.js', 'webpack.config.js'
run "yarn add rails_admin@#{RailsAdmin::Version.js}"
template 'rails_admin.js', 'app/javascript/rails_admin.js'
webpack_config = File.join(destination_root, 'webpack.config.js')
marker = %r{application: ["']./app/javascript/application.js["']}
if File.exist?(webpack_config) && File.read(webpack_config) =~ marker
insert_into_file 'webpack.config.js', %(,\n rails_admin: "./app/javascript/rails_admin.js"), after: marker
else
say 'Add `rails_admin: "./app/javascript/rails_admin.js"` to the entry section in your webpack.config.js.', :red
end
setup_css({'build' => 'webpack --config webpack.config.js'})
end

def configure_for_importmap
run "yarn add rails_admin@#{RailsAdmin::Version.js}"
template 'rails_admin.js', 'app/javascript/rails_admin.js'
require_relative 'importmap_formatter'
add_file 'config/importmap.rails_admin.rb', ImportmapFormatter.new.format
setup_css
end

def setup_css(additional_script_entries = {})
gem 'cssbundling-rails'
rake 'css:install:sass'

@fa_font_path = '.'
template 'rails_admin.scss.erb', 'app/assets/stylesheets/rails_admin.scss'
asset_config = %{Rails.application.config.assets.paths << Rails.root.join("node_modules/@fortawesome/fontawesome-free/webfonts")\n}
if File.exist? File.join(destination_root, 'config/initializers/assets.rb')
append_to_file 'config/initializers/assets.rb', asset_config
else
add_file 'config/initializers/assets.rb', asset_config
end
add_scripts(additional_script_entries.merge({'build:css' => 'sass ./app/assets/stylesheets/rails_admin.scss:./app/assets/builds/rails_admin.css --no-source-map --load-path=node_modules'}))
end

def add_scripts(entries)
display 'Add scripts to package.json'
package = begin
JSON.parse(File.read(File.join(destination_root, 'package.json')))
rescue Errno::ENOENT, JSON::ParserError
{}
end
if package['scripts'] && (package['scripts'].keys & entries.keys).any?
say <<-MESSAGE.gsub(/^ {10}/, ''), :red
You need to merge "scripts": #{JSON.pretty_generate(entries)} into the existing scripts in your package.json .
Taking 'build:css' as an example, if you're already have application.sass.css for the sass build, the resulting script would look like:
sass ./app/assets/stylesheets/application.sass.scss:./app/assets/builds/application.css ./app/assets/stylesheets/rails_admin.scss:./app/assets/builds/rails_admin.css --no-source-map --load-path=node_modules
MESSAGE
else
package['scripts'] ||= {}
entries.each do |entry, build_script|
package['scripts'][entry] = build_script
end
add_file 'package.json', JSON.pretty_generate(package)
end
end
end
end
1 change: 1 addition & 0 deletions lib/generators/rails_admin/templates/rails_admin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import "rails_admin/src/rails_admin/base";
2 changes: 0 additions & 2 deletions lib/generators/rails_admin/templates/rails_admin.js.erb

This file was deleted.

1 change: 0 additions & 1 deletion lib/generators/rails_admin/templates/rails_admin.scss

This file was deleted.

1 change: 1 addition & 0 deletions lib/generators/rails_admin/templates/rails_admin.scss.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<%= instance_variable_defined?(:@fa_font_path) ? %{$fa-font-path: "#{@fa_font_path}";\n} : '' %>@import "rails_admin/src/rails_admin/styles/base";
2 changes: 2 additions & 0 deletions lib/generators/rails_admin/templates/rails_admin.webpacker.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import "rails_admin/src/rails_admin/base";
import "../stylesheets/rails_admin.scss";
29 changes: 0 additions & 29 deletions lib/generators/rails_admin/templates/webpack.config.js

This file was deleted.

43 changes: 26 additions & 17 deletions lib/rails_admin/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,9 @@ module RailsAdmin
class Engine < Rails::Engine
isolate_namespace RailsAdmin

config.action_dispatch.rescue_responses['RailsAdmin::ActionNotAllowed'] = :forbidden
attr_accessor :importmap

initializer 'RailsAdmin precompile hook', group: :all do |app|
if defined?(Sprockets)
app.config.assets.precompile += %w[
rails_admin/application.js
rails_admin/application.css
]
app.config.assets.paths << RailsAdmin::Engine.root.join('src')
require 'rails_admin/support/esmodule_preprocessor'
Sprockets.register_preprocessor 'application/javascript', RailsAdmin::ESModulePreprocessor
end
end
config.action_dispatch.rescue_responses['RailsAdmin::ActionNotAllowed'] = :forbidden

initializer 'RailsAdmin reload config in development' do |app|
config.initializer_path = app.root.join('config/initializers/rails_admin.rb')
Expand All @@ -45,6 +35,30 @@ class Engine < Rails::Engine
end
end

initializer 'RailsAdmin apply configuration', after: :eager_load! do |app|
RailsAdmin::Config.initialize!

# Force route reload, since it doesn't reflect RailsAdmin action configuration yet
app.reload_routes!
end

initializer 'RailsAdmin precompile hook', group: :all, after: 'RailsAdmin apply configuration' do |app|
case RailsAdmin.config.asset_source
Copy link

Choose a reason for hiding this comment

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

@mshibuya
Running rails rails_admin:install will fail. You need to check if the class exists and return an importmap.

def asset_source

I made the following modifications. I hope this will be helpful.

#3495

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't understand the problem, what did you do and what did you get?
Did you try to do config.asset_source = :importmap without installing importmap-rails?

Copy link

@yubele yubele Mar 12, 2022

Choose a reason for hiding this comment

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

Did you try to do config.asset_source = :importmap without installing importmap-rails?

importmap-rails has been pre-installed.

config.asset_source = :importmap

I believe the above refers to RailsAdmin.config.asset_source.

I believe this problem occurs because $ bin/rails rails_admin:install has not yet created config/initializers/rails_admin.rb at the stage of running $ bin/rails rails_admin:install.

Here are the steps I followed

  1. $ bundle add importmap-rails.
    2.$ . /bin/rails importmap:install.
    3.$ bundle add rails_admin --branch importmap --github railsadminteam/rails_admin
  2. $ bin/rails rails_admin:install.

I got the following error in 4.

[Warning] After upgrading RailsAdmin to 3.x you haven't set asset_source yet, using :sprockets as the default.
To suppress this message, run 'rails rails_admin:install' to setup the asset delivery method suitable to you.
/usr/local/bundle/bundler/gems/rails_admin-d073c4dfd315/lib/rails_admin/engine.rb:53:in `block in <class:Engine>': uninitialized constant RailsAdmin::Engine::Sprockets (NameError)

        Sprockets.register_preprocessor 'application/javascript', RailsAdmin::ESModulePreprocessor
        ^^^^^^^^^
Did you mean?  Socket
               Process
               IPSocket

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah I understood now, thanks for letting me know. Fixed by 705fd32 👍

when :sprockets
if defined?(Sprockets)
app.config.assets.precompile += %w[
rails_admin/application.js
rails_admin/application.css
]
app.config.assets.paths << RailsAdmin::Engine.root.join('src')
require 'rails_admin/support/esmodule_preprocessor'
Sprockets.register_preprocessor 'application/javascript', RailsAdmin::ESModulePreprocessor
end
when :importmap
self.importmap = Importmap::Map.new.draw(app.root.join('config/importmap.rails_admin.rb'))
end
end

# Check for required middlewares, users may forget to use them in Rails API mode
config.after_initialize do |app|
has_session_store = app.config.middleware.to_a.any? do |m|
Expand All @@ -68,11 +82,6 @@ class Engine < Rails::Engine
ERROR
end

RailsAdmin::Config.initialize!

# Force route reload, since it doesn't reflect RailsAdmin action configuration yet
app.reload_routes!

RailsAdmin::Version.warn_with_js_version
end
end
Expand Down
2 changes: 2 additions & 0 deletions lib/rails_admin/support/esmodule_preprocessor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ def self.call(input)
def initialize; end

def call(input)
return unless RailsAdmin.config.asset_source == :sprockets

data = input[:data]

if input[:filename].start_with? RailsAdmin::Engine.root.join('src').to_s
Expand Down
3 changes: 3 additions & 0 deletions spec/dummy_app/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
# Ignore bundler config
/.bundle

/app/assets/builds/*
!/app/assets/builds/.keep

# Ignore the default SQLite database.
/db/*.sqlite3

Expand Down
16 changes: 5 additions & 11 deletions spec/dummy_app/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@

source 'https://rubygems.org'

gem 'rails', '>= 6.0.0'
gem 'webpacker', require: false
gem 'rails', '>= 7.0.0'

group :active_record do
platforms :jruby do
Expand All @@ -22,22 +21,17 @@ group :active_record do
gem 'paper_trail', '>= 12.0'
end

group :mongoid do
gem 'carrierwave-mongoid', '>= 0.6.3', require: 'carrierwave/mongoid'
gem 'kaminari-mongoid'
gem 'mongoid', ['>= 6.0', '< 8']
gem 'mongoid-paperclip', '>= 0.0.8', require: 'mongoid_paperclip'
gem 'shrine-mongoid', '~> 1.0'
end

gem 'carrierwave', '>= 2.0.0.rc', '< 3.0'
gem 'devise', '>= 3.2', github: 'heartcombo/devise', branch: 'main'
gem 'cssbundling-rails'
gem 'devise', '>= 3.2'
gem 'dragonfly', '~> 1.0'
gem 'importmap-rails', require: false
gem 'mini_magick', '>= 3.4'
gem 'mlb', '>= 0.7', github: 'mshibuya/mlb', branch: 'ruby-3'
gem 'paperclip', '>= 3.4'
gem 'rails_admin', path: '../../'
gem 'shrine', '~> 3.0'
gem 'webpacker', require: false
gem 'webrick', '~> 1.7'

# Gems used only for assets and not required
Expand Down
Loading