From 0034b3fb8241f1b1df69d0e08d8bce940b4561a8 Mon Sep 17 00:00:00 2001 From: Lukas 'ai-pi' Eipert <leipert@gitlab.com> Date: Wed, 10 Apr 2024 19:46:25 +0000 Subject: [PATCH] Consume icon.json files as assets Instead of loading `icons.json` and `file_icons/file_icons.json` from `node_modules/@gitlab/svgs/dist`, we can also make them assets and consume them from the asset pipeline. This means that if assets are pre-compiled inside of `compile-test-assets`, they will be included in the asset bundle and our rspec tests can pass _without_ relying on the `node_modules` folder. --- .gitlab/ci/frontend.gitlab-ci.yml | 2 -- app/helpers/icons_helper.rb | 15 +++++++++++++-- config/application.rb | 1 + lib/tasks/gitlab/assets.rake | 1 + spec/helpers/icons_helper_spec.rb | 22 +++++++++++++++------- 5 files changed, 30 insertions(+), 11 deletions(-) diff --git a/.gitlab/ci/frontend.gitlab-ci.yml b/.gitlab/ci/frontend.gitlab-ci.yml index d46bbed3db8a5..2fedf579e0560 100644 --- a/.gitlab/ci/frontend.gitlab-ci.yml +++ b/.gitlab/ci/frontend.gitlab-ci.yml @@ -74,8 +74,6 @@ compile-test-assets: paths: - public/assets/ - config/helpers/tailwind/ # Assets created during tailwind compilation - - node_modules/@gitlab/svgs/dist/icons.json # app/helpers/icons_helper.rb uses this file - - node_modules/@gitlab/svgs/dist/file_icons/file_icons.json # app/helpers/icons_helper.rb uses this file - "${WEBPACK_COMPILE_LOG_PATH}" when: always diff --git a/app/helpers/icons_helper.rb b/app/helpers/icons_helper.rb index 7cf7f9db75e64..a3704dc6aa8c0 100644 --- a/app/helpers/icons_helper.rb +++ b/app/helpers/icons_helper.rb @@ -184,13 +184,24 @@ def unknown_file_icon_sprite(icon_name) def known_sprites return if Rails.env.production? - @known_sprites ||= Gitlab::Json.parse(File.read(Rails.root.join('node_modules/@gitlab/svgs/dist/icons.json')))['icons'] + @known_sprites ||= parse_sprite_definition('icons.json')['icons'] end def known_file_icon_sprites return if Rails.env.production? - @known_file_icon_sprites ||= Gitlab::Json.parse(File.read(Rails.root.join('node_modules/@gitlab/svgs/dist/file_icons/file_icons.json')))['icons'] + @known_file_icon_sprites ||= parse_sprite_definition('file_icons/file_icons.json')['icons'] + end + + def parse_sprite_definition(sprite_definition) + Gitlab::Json.parse( + Rails.application + .assets_manifest + .find_sources(sprite_definition) + .first + .to_s + .force_encoding('UTF-8') + ) end def memoized_icon(key) diff --git a/config/application.rb b/config/application.rb index f5be077f0c54c..81cd1314c79cd 100644 --- a/config/application.rb +++ b/config/application.rb @@ -398,6 +398,7 @@ class Application < Rails::Application config.assets.precompile << "icons.svg" config.assets.precompile << "icons.json" config.assets.precompile << "file_icons/file_icons.svg" + config.assets.precompile << "file_icons/file_icons.json" config.assets.precompile << "illustrations/*.svg" config.assets.precompile << "illustrations/*.png" diff --git a/lib/tasks/gitlab/assets.rake b/lib/tasks/gitlab/assets.rake index fa294761b43fb..5a6e6996fff6f 100644 --- a/lib/tasks/gitlab/assets.rake +++ b/lib/tasks/gitlab/assets.rake @@ -21,6 +21,7 @@ module Tasks # or have a direct impact on asset compilation (e.g. scss) and therefore # we should compile when these change RAILS_ASSET_FILES = %w[ + config/application.rb Gemfile Gemfile.lock ].freeze diff --git a/spec/helpers/icons_helper_spec.rb b/spec/helpers/icons_helper_spec.rb index 1a882c8a894a3..f3732fa69abdb 100644 --- a/spec/helpers/icons_helper_spec.rb +++ b/spec/helpers/icons_helper_spec.rb @@ -66,28 +66,36 @@ end describe 'non existing icon' do + let(:helper) do + Class.new do + include IconsHelper + end.new + end + non_existing = 'non_existing_icon_sprite' it 'raises in development mode' do stub_rails_env('development') - expect { sprite_icon(non_existing) }.to raise_error(ArgumentError, /is not a known icon/) - expect { sprite_icon(non_existing, file_icon: true) }.to raise_error(ArgumentError, /is not a known icon/) + expect(helper).to receive(:parse_sprite_definition).with('icons.json').once.and_call_original + expect(helper).to receive(:parse_sprite_definition).with('file_icons/file_icons.json').once.and_call_original + expect { helper.sprite_icon(non_existing) }.to raise_error(ArgumentError, /is not a known icon/) + expect { helper.sprite_icon(non_existing, file_icon: true) }.to raise_error(ArgumentError, /is not a known icon/) end it 'raises in test mode' do stub_rails_env('test') - expect { sprite_icon(non_existing) }.to raise_error(ArgumentError, /is not a known icon/) - expect { sprite_icon(non_existing, file_icon: true) }.to raise_error(ArgumentError, /is not a known icon/) + expect(helper).to receive(:parse_sprite_definition).with('icons.json').once.and_call_original + expect(helper).to receive(:parse_sprite_definition).with('file_icons/file_icons.json').once.and_call_original + expect { helper.sprite_icon(non_existing) }.to raise_error(ArgumentError, /is not a known icon/) + expect { helper.sprite_icon(non_existing, file_icon: true) }.to raise_error(ArgumentError, /is not a known icon/) end it 'does not raise in production mode' do stub_rails_env('production') - expect_file_not_to_read(Rails.root.join('node_modules/@gitlab/svgs/dist/icons.json')) - expect_file_not_to_read(Rails.root.join('node_modules/@gitlab/svgs/dist/file_icons/file_icons.json')) - + expect(helper).not_to receive(:parse_sprite_definition) expect { sprite_icon(non_existing) }.not_to raise_error expect { sprite_icon(non_existing, file_icon: true) }.not_to raise_error end -- GitLab