From 05483ed63236b0f7d37adfe710264e679db57950 Mon Sep 17 00:00:00 2001 From: Stanislav Lashmanov Date: Wed, 1 Nov 2023 15:45:51 +0400 Subject: [PATCH 1/9] Support Vite in feature tests --- app/controllers/application_controller.rb | 2 +- app/helpers/vite_helper.rb | 12 +++++------- app/helpers/webpack_helper.rb | 6 ++++-- app/views/layouts/_head.html.haml | 2 +- config/gitlab.yml.example | 8 +++++++- config/vite.json | 14 ++------------ spec/helpers/webpack_helper_spec.rb | 2 +- 7 files changed, 21 insertions(+), 25 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6739fc57a1f012..f4d9d616851cfe 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -116,7 +116,7 @@ def self.endpoint_id_for_action(action_name) content_security_policy do |p| next if p.directives.blank? - if Rails.env.development? && Feature.enabled?(:vite) + if helpers.vite_enabled? vite_host = ViteRuby.instance.config.host vite_port = ViteRuby.instance.config.port vite_origin = "#{vite_host}:#{vite_port}" diff --git a/app/helpers/vite_helper.rb b/app/helpers/vite_helper.rb index 5096d3649b7bf9..e96cdcc2b62e83 100644 --- a/app/helpers/vite_helper.rb +++ b/app/helpers/vite_helper.rb @@ -1,13 +1,11 @@ # frozen_string_literal: true module ViteHelper - private + def vite_enabled? + return false unless Rails.env.development? || Rails.env.test? - def vite_enabled - Feature.enabled?(:vite) && !Rails.env.test? && vite_running - end - - def vite_running - ViteRuby.instance.dev_server_running? + Gitlab.config.vite&.enabled + rescue GitlabSettings::MissingSetting + Feature.enabled?(:vite) end end diff --git a/app/helpers/webpack_helper.rb b/app/helpers/webpack_helper.rb index 928741687980a2..e1e2d4581ac470 100644 --- a/app/helpers/webpack_helper.rb +++ b/app/helpers/webpack_helper.rb @@ -16,7 +16,7 @@ def prefetch_link_tag(source) end def webpack_bundle_tag(bundle) - if vite_running + if vite_enabled? vite_javascript_tag bundle else javascript_include_tag(*webpack_entrypoint_paths(bundle)) @@ -24,6 +24,8 @@ def webpack_bundle_tag(bundle) end def webpack_preload_asset_tag(asset, options = {}) + return if vite_enabled? + path = Gitlab::Webpack::Manifest.asset_paths(asset).first if options.delete(:prefetch) @@ -38,7 +40,7 @@ def webpack_preload_asset_tag(asset, options = {}) end def webpack_controller_bundle_tags - return if Feature.enabled?(:vite) && !Rails.env.test? + return if vite_enabled? chunks = [] diff --git a/app/views/layouts/_head.html.haml b/app/views/layouts/_head.html.haml index 41f663c7c06508..2c97df90110dac 100644 --- a/app/views/layouts/_head.html.haml +++ b/app/views/layouts/_head.html.haml @@ -50,7 +50,7 @@ = webpack_bundle_tag 'legacy_sentry' = webpack_bundle_tag 'performance_bar' if performance_bar_enabled? - - if vite_enabled + - if vite_enabled? %meta{ name: 'controller-path', content: controller_full_path } - if Rails.env.development? = vite_client_tag diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 5002e9e24bf311..a8f18447ef0b20 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -1298,7 +1298,7 @@ production: &base ## Webpack settings # If enabled, this will tell rails to serve frontend assets from the webpack-dev-server running - # on a given port instead of serving directly from /assets/webpack. This is only indended for use + # on a given port instead of serving directly from /assets/webpack. This is only intended for use # in development. webpack: # dev_server: @@ -1306,6 +1306,12 @@ production: &base # host: localhost # port: 3808 + ## Vite settings + # If enabled, this will tell rails to serve frontend assets from the vite dev server. + # This is only intended for use in development. + vite: + # enabled: true + ## Monitoring # Built in monitoring settings monitoring: diff --git a/config/vite.json b/config/vite.json index b428b0daec022d..92e67d256818d9 100644 --- a/config/vite.json +++ b/config/vite.json @@ -2,17 +2,7 @@ "all": { "sourceCodeDir": "app/assets", "entrypointsDir": "javascripts/entrypoints", - "devServerConnectTimeout": 3 - }, - "development": { - "autoBuild": true, - "publicOutputDir": "vite-dev", - "host": "localhost", - "port": 3038 - }, - "test": { - "autoBuild": true, - "publicOutputDir": "vite-test", - "port": 3037 + "devServerConnectTimeout": 3, + "publicOutputDir": "vite-dev" } } diff --git a/spec/helpers/webpack_helper_spec.rb b/spec/helpers/webpack_helper_spec.rb index 23585c4723998a..8cbc4db9108bf8 100644 --- a/spec/helpers/webpack_helper_spec.rb +++ b/spec/helpers/webpack_helper_spec.rb @@ -43,7 +43,7 @@ stub_feature_flags(vite: true) allow(helper).to receive(:vite_javascript_tag).and_return('vite') - allow(helper).to receive(:vite_running).and_return(true) + allow(helper).to receive(:vite_enabled?).and_return(true) end describe '#webpack_bundle_tag' do -- GitLab From cb95dca979d396dd7c4480637f75f2f447ff5a65 Mon Sep 17 00:00:00 2001 From: Lukas Eipert Date: Mon, 6 Nov 2023 16:09:14 +0100 Subject: [PATCH 2/9] Configure vite_ruby via config file This allows us to pass the port and whether vite_ruby is actually enabled from the GDK to GitLab Rails and feature specs. --- .gitignore | 1 + app/helpers/vite_helper.rb | 4 ++-- config/gitlab.yml.example | 8 +------- config/vite.rb | 21 +++++++++++++++++++++ 4 files changed, 25 insertions(+), 9 deletions(-) create mode 100644 config/vite.rb diff --git a/.gitignore b/.gitignore index 914daba7994efc..cae30d90c044ab 100644 --- a/.gitignore +++ b/.gitignore @@ -114,3 +114,4 @@ tags.temp # https://vitejs.dev/guide/env-and-mode.html#env-files *.local +config/vite.gdk.json diff --git a/app/helpers/vite_helper.rb b/app/helpers/vite_helper.rb index e96cdcc2b62e83..097858ca159c51 100644 --- a/app/helpers/vite_helper.rb +++ b/app/helpers/vite_helper.rb @@ -4,8 +4,8 @@ module ViteHelper def vite_enabled? return false unless Rails.env.development? || Rails.env.test? - Gitlab.config.vite&.enabled - rescue GitlabSettings::MissingSetting + return Gitlab::Utils.to_boolean(ViteRuby.env['VITE_ENABLED']) if ViteRuby.env['VITE_ENABLED'].present? + Feature.enabled?(:vite) end end diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index a8f18447ef0b20..5002e9e24bf311 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -1298,7 +1298,7 @@ production: &base ## Webpack settings # If enabled, this will tell rails to serve frontend assets from the webpack-dev-server running - # on a given port instead of serving directly from /assets/webpack. This is only intended for use + # on a given port instead of serving directly from /assets/webpack. This is only indended for use # in development. webpack: # dev_server: @@ -1306,12 +1306,6 @@ production: &base # host: localhost # port: 3808 - ## Vite settings - # If enabled, this will tell rails to serve frontend assets from the vite dev server. - # This is only intended for use in development. - vite: - # enabled: true - ## Monitoring # Built in monitoring settings monitoring: diff --git a/config/vite.rb b/config/vite.rb new file mode 100644 index 00000000000000..946d1e880681a6 --- /dev/null +++ b/config/vite.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'yaml' + +def load_gdk_vite_config + config = YAML.safe_load_file("#{__dir__}/vite.gdk.json") + + return unless config['enabled'] + + ViteRuby.env['VITE_ENABLED'] = 'true' + + ViteRuby.configure( + port: config['port'] + ) +rescue StandardError + # We will probably end up here, if the file doesn't exist or cannot be parsed. + # which is fine by us +end + +ViteRuby.env['VITE_ENABLED'] = 'false' +load_gdk_vite_config -- GitLab From 1583201cbeba3a5745a6ed4da063115f8df22679 Mon Sep 17 00:00:00 2001 From: Lukas 'Eipi' Eipert Date: Tue, 7 Nov 2023 12:57:32 +0000 Subject: [PATCH 3/9] Make gitignore line match --- .gitignore | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index cae30d90c044ab..4eb7ce8303d70f 100644 --- a/.gitignore +++ b/.gitignore @@ -113,5 +113,4 @@ tags.temp # Vite uses dotenv and suggests to ignore local-only env files. See # https://vitejs.dev/guide/env-and-mode.html#env-files *.local - -config/vite.gdk.json +/config/vite.gdk.json -- GitLab From 90d51a168b9e98328f42dc7e50861a22c534f833 Mon Sep 17 00:00:00 2001 From: Lukas Eipert Date: Tue, 7 Nov 2023 14:23:08 +0100 Subject: [PATCH 4/9] Only set VITE_ENABLED if file exists --- config/vite.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/config/vite.rb b/config/vite.rb index 946d1e880681a6..4b484062b14f89 100644 --- a/config/vite.rb +++ b/config/vite.rb @@ -5,17 +5,16 @@ def load_gdk_vite_config config = YAML.safe_load_file("#{__dir__}/vite.gdk.json") - return unless config['enabled'] + ViteRuby.env['VITE_ENABLED'] = (config['enabled']).to_s - ViteRuby.env['VITE_ENABLED'] = 'true' + return unless config['enabled'] ViteRuby.configure( port: config['port'] ) rescue StandardError # We will probably end up here, if the file doesn't exist or cannot be parsed. - # which is fine by us + # This might mean an older branch end -ViteRuby.env['VITE_ENABLED'] = 'false' load_gdk_vite_config -- GitLab From 0fa54dabbae19831d3a528894ce683f76d3b26ff Mon Sep 17 00:00:00 2001 From: Stanislav Lashmanov Date: Tue, 7 Nov 2023 19:13:05 +0400 Subject: [PATCH 5/9] Protect feature tests from Vite in CI Add a link to the feature flag sunsetting issue --- app/helpers/vite_helper.rb | 7 +++++-- spec/spec_helper.rb | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/helpers/vite_helper.rb b/app/helpers/vite_helper.rb index 097858ca159c51..ec63fde4d28bc8 100644 --- a/app/helpers/vite_helper.rb +++ b/app/helpers/vite_helper.rb @@ -2,10 +2,13 @@ module ViteHelper def vite_enabled? + # vite is not production ready yet return false unless Rails.env.development? || Rails.env.test? - + # Enable vite if explicitly turned on in the GDK return Gitlab::Utils.to_boolean(ViteRuby.env['VITE_ENABLED']) if ViteRuby.env['VITE_ENABLED'].present? - Feature.enabled?(:vite) + # Enable vite the legacy way (in case GDK hasn't been updated) + # This is going to be removed with https://gitlab.com/gitlab-org/gitlab/-/issues/431041 + Rails.env.development? ? Feature.enabled?(:vite) : false end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2dd4e92eee9b3c..4876acfce8a3b2 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -327,6 +327,7 @@ # Postgres is the primary data source, and ClickHouse only when enabled in certain cases. stub_feature_flags(clickhouse_data_collection: false) + # This is going to be removed with https://gitlab.com/gitlab-org/gitlab/-/issues/431041 stub_feature_flags(vite: false) else unstub_all_feature_flags -- GitLab From 8ebac0e8f728c1c8d25b5336426fbb6b7ff7f6a5 Mon Sep 17 00:00:00 2001 From: Stanislav Lashmanov Date: Wed, 8 Nov 2023 18:44:40 +0400 Subject: [PATCH 6/9] Switch VITE_ENABLED from string to a boolean Always convert Vite port to a number Add stricter checks for errors during Vite initialization --- app/helpers/vite_helper.rb | 2 +- config/vite.rb | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/helpers/vite_helper.rb b/app/helpers/vite_helper.rb index ec63fde4d28bc8..e1519c5ec764a4 100644 --- a/app/helpers/vite_helper.rb +++ b/app/helpers/vite_helper.rb @@ -5,7 +5,7 @@ def vite_enabled? # vite is not production ready yet return false unless Rails.env.development? || Rails.env.test? # Enable vite if explicitly turned on in the GDK - return Gitlab::Utils.to_boolean(ViteRuby.env['VITE_ENABLED']) if ViteRuby.env['VITE_ENABLED'].present? + return ViteRuby.env['VITE_ENABLED'] if ViteRuby.env.key?('VITE_ENABLED') # Enable vite the legacy way (in case GDK hasn't been updated) # This is going to be removed with https://gitlab.com/gitlab-org/gitlab/-/issues/431041 diff --git a/config/vite.rb b/config/vite.rb index 4b484062b14f89..7a224b56d40135 100644 --- a/config/vite.rb +++ b/config/vite.rb @@ -5,14 +5,15 @@ def load_gdk_vite_config config = YAML.safe_load_file("#{__dir__}/vite.gdk.json") - ViteRuby.env['VITE_ENABLED'] = (config['enabled']).to_s + enabled = config.fetch('enabled', false) + ViteRuby.env['VITE_ENABLED'] = enabled - return unless config['enabled'] + return unless enabled ViteRuby.configure( - port: config['port'] + port: Integer(config.fetch('port', 3036)) ) -rescue StandardError +rescue YAML::ParseError, IOError # We will probably end up here, if the file doesn't exist or cannot be parsed. # This might mean an older branch end -- GitLab From 0c71f00b0b8991d50cc31356c398c9a47337dbf5 Mon Sep 17 00:00:00 2001 From: Stanislav Lashmanov Date: Thu, 9 Nov 2023 13:14:59 +0400 Subject: [PATCH 7/9] Prevent Vite from running in production mode --- app/helpers/vite_helper.rb | 2 +- config/vite.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/helpers/vite_helper.rb b/app/helpers/vite_helper.rb index e1519c5ec764a4..a8ecfe51f28fa0 100644 --- a/app/helpers/vite_helper.rb +++ b/app/helpers/vite_helper.rb @@ -3,7 +3,7 @@ module ViteHelper def vite_enabled? # vite is not production ready yet - return false unless Rails.env.development? || Rails.env.test? + return false if Rails.env.production? # Enable vite if explicitly turned on in the GDK return ViteRuby.env['VITE_ENABLED'] if ViteRuby.env.key?('VITE_ENABLED') diff --git a/config/vite.rb b/config/vite.rb index 7a224b56d40135..c3898f4b34cc1e 100644 --- a/config/vite.rb +++ b/config/vite.rb @@ -18,4 +18,4 @@ def load_gdk_vite_config # This might mean an older branch end -load_gdk_vite_config +load_gdk_vite_config unless ENV['RAILS_ENV'] == 'production' -- GitLab From 325dca8f668b9f2e09bad0e70079d3c78af396dc Mon Sep 17 00:00:00 2001 From: Stanislav Lashmanov Date: Thu, 9 Nov 2023 17:13:34 +0400 Subject: [PATCH 8/9] Test Vite GDK integration --- config/vite.rb | 19 ++---------- lib/vite_gdk.rb | 24 +++++++++++++++ spec/lib/vite_gdk_spec.rb | 63 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 17 deletions(-) create mode 100644 lib/vite_gdk.rb create mode 100644 spec/lib/vite_gdk_spec.rb diff --git a/config/vite.rb b/config/vite.rb index c3898f4b34cc1e..23495825b6f972 100644 --- a/config/vite.rb +++ b/config/vite.rb @@ -1,21 +1,6 @@ # frozen_string_literal: true require 'yaml' +require_relative '../lib/vite_gdk' -def load_gdk_vite_config - config = YAML.safe_load_file("#{__dir__}/vite.gdk.json") - - enabled = config.fetch('enabled', false) - ViteRuby.env['VITE_ENABLED'] = enabled - - return unless enabled - - ViteRuby.configure( - port: Integer(config.fetch('port', 3036)) - ) -rescue YAML::ParseError, IOError - # We will probably end up here, if the file doesn't exist or cannot be parsed. - # This might mean an older branch -end - -load_gdk_vite_config unless ENV['RAILS_ENV'] == 'production' +ViteGdk.load_gdk_vite_config diff --git a/lib/vite_gdk.rb b/lib/vite_gdk.rb new file mode 100644 index 00000000000000..10bbe43be173e4 --- /dev/null +++ b/lib/vite_gdk.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module ViteGdk + def self.load_gdk_vite_config + # can't use Rails.env.production? here because this file is required outside of Gitlab app instance + return if ENV['RAILS_ENV'] == 'production' + + return unless File.exist?(vite_gdk_config_path) + + config = YAML.safe_load_file(vite_gdk_config_path) + enabled = config.fetch('enabled', false) + ViteRuby.env['VITE_ENABLED'] = enabled + + return unless enabled + + ViteRuby.configure( + port: Integer(config.fetch('port', 3036)) + ) + end + + def self.vite_gdk_config_path + File.join(__dir__, '../config/vite.gdk.json') + end +end diff --git a/spec/lib/vite_gdk_spec.rb b/spec/lib/vite_gdk_spec.rb new file mode 100644 index 00000000000000..8a629a3126c72d --- /dev/null +++ b/spec/lib/vite_gdk_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'spec_helper' + +VITE_GDK_CONFIG_FILEPATH = "config/vite.gdk.json" + +RSpec.describe ViteGdk, feature_category: :tooling do + before do + allow(ViteRuby).to receive(:configure) + allow(ViteRuby.env).to receive(:[]=) + allow(YAML).to receive(:safe_load_file) + end + + describe '#load_gdk_vite_config' do + context 'when not in production environment' do + before do + stub_env('RAILS_ENV', nil) + end + + context 'when it loads file successfully' do + it 'configures ViteRuby' do + expect(File).to receive(:exist?) do |file_path| + expect(file_path).to end_with(VITE_GDK_CONFIG_FILEPATH) + end.and_return(true) + expect(YAML).to receive(:safe_load_file) do |file_path| + expect(file_path).to end_with(VITE_GDK_CONFIG_FILEPATH) + end.and_return('enabled' => true, 'port' => 3036) + expect(ViteRuby).to receive(:configure).with(port: 3036) + expect(ViteRuby.env).to receive(:[]=).with('VITE_ENABLED', true) + + described_class.load_gdk_vite_config + end + end + + context 'when config file is missing' do + it 'does nothing' do + expect(File).to receive(:exist?) do |file_path| + expect(file_path).to end_with(VITE_GDK_CONFIG_FILEPATH) + end.and_return(false) + expect(ViteRuby).not_to receive(:configure) + expect(ViteRuby.env).not_to receive(:[]=).with('VITE_ENABLED', false) + expect(ViteRuby.env).not_to receive(:[]=).with('VITE_ENABLED', true) + + described_class.load_gdk_vite_config + end + end + end + + context 'when in production environment' do + before do + stub_env('RAILS_ENV', 'production') + end + + it 'does not load and configure ViteRuby' do + expect(YAML).not_to receive(:safe_load_file) + expect(ViteRuby).not_to receive(:configure) + expect(ViteRuby.env).not_to receive(:[]=).with('VITE_ENABLED') + + described_class.load_gdk_vite_config + end + end + end +end -- GitLab From 6d8c4774be4c88965c6714cc4a4fab4e4097302f Mon Sep 17 00:00:00 2001 From: Lukas 'Eipi' Eipert Date: Thu, 9 Nov 2023 16:52:42 +0000 Subject: [PATCH 9/9] Move back to 3038 as the "default" branch Let's ensure that's aligned everywhere --- config/vite.json | 5 +++-- lib/vite_gdk.rb | 2 +- spec/lib/vite_gdk_spec.rb | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/config/vite.json b/config/vite.json index 92e67d256818d9..14b5da38ab2612 100644 --- a/config/vite.json +++ b/config/vite.json @@ -2,7 +2,8 @@ "all": { "sourceCodeDir": "app/assets", "entrypointsDir": "javascripts/entrypoints", - "devServerConnectTimeout": 3, - "publicOutputDir": "vite-dev" + "port": 3038, + "publicOutputDir": "vite-dev", + "devServerConnectTimeout": 3 } } diff --git a/lib/vite_gdk.rb b/lib/vite_gdk.rb index 10bbe43be173e4..e4dd85c9c7f3ae 100644 --- a/lib/vite_gdk.rb +++ b/lib/vite_gdk.rb @@ -14,7 +14,7 @@ def self.load_gdk_vite_config return unless enabled ViteRuby.configure( - port: Integer(config.fetch('port', 3036)) + port: Integer(config.fetch('port', 3038)) ) end diff --git a/spec/lib/vite_gdk_spec.rb b/spec/lib/vite_gdk_spec.rb index 8a629a3126c72d..fed2ca6921faeb 100644 --- a/spec/lib/vite_gdk_spec.rb +++ b/spec/lib/vite_gdk_spec.rb @@ -24,8 +24,8 @@ end.and_return(true) expect(YAML).to receive(:safe_load_file) do |file_path| expect(file_path).to end_with(VITE_GDK_CONFIG_FILEPATH) - end.and_return('enabled' => true, 'port' => 3036) - expect(ViteRuby).to receive(:configure).with(port: 3036) + end.and_return('enabled' => true, 'port' => 3038) + expect(ViteRuby).to receive(:configure).with(port: 3038) expect(ViteRuby.env).to receive(:[]=).with('VITE_ENABLED', true) described_class.load_gdk_vite_config -- GitLab