From c822df4aebfe36c07456ce9915d3515c618a3ba6 Mon Sep 17 00:00:00 2001
From: Stanislav Lashmanov <slashmanov@gitlab.com>
Date: Wed, 27 Mar 2024 12:36:05 +0000
Subject: [PATCH] Handle controller paths with non-existant entry points
We have the controller `profiles/keys` with the action `show`, so it
tries to load: `pages.profiles.keys.show.js` which doesn't exist. The
following entry points do exist:
pages/profiles/keys/show/index.js <== does not exist
pages/profiles/keys/index.js <== does exist
pages/profiles/index.js <== does exist
So, it should have tried loading the other two, but didn't. By moving
the "backtracking" logic to the backend, we can try loading all of those
virtual entry points.
---
app/assets/javascripts/entrypoints/main.js | 5 -
app/assets/javascripts/entrypoints/main_ee.js | 5 -
app/assets/javascripts/entrypoints/main_jh.js | 5 -
app/assets/javascripts/run_modules.js | 48 -------
app/helpers/application_helper.rb | 9 --
app/helpers/vite_helper.rb | 13 ++
app/views/layouts/_head.html.haml | 7 +-
config/webpack.config.js | 124 ++++++------------
config/webpack.helpers.js | 69 ++++++++++
package.json | 3 +-
spec/helpers/application_helper_spec.rb | 34 -----
spec/helpers/vite_helper_spec.rb | 26 ++++
vite.config.js | 35 +++++
13 files changed, 184 insertions(+), 199 deletions(-)
delete mode 100644 app/assets/javascripts/entrypoints/main_ee.js
delete mode 100644 app/assets/javascripts/entrypoints/main_jh.js
delete mode 100644 app/assets/javascripts/run_modules.js
create mode 100644 config/webpack.helpers.js
create mode 100644 spec/helpers/vite_helper_spec.rb
diff --git a/app/assets/javascripts/entrypoints/main.js b/app/assets/javascripts/entrypoints/main.js
index 003a2f31829aa..b627bb195a939 100644
--- a/app/assets/javascripts/entrypoints/main.js
+++ b/app/assets/javascripts/entrypoints/main.js
@@ -1,6 +1 @@
import '../main';
-import { runModules } from '~/run_modules';
-
-const modules = import.meta.glob('../pages/**/index.js');
-
-runModules(modules, 'CE');
diff --git a/app/assets/javascripts/entrypoints/main_ee.js b/app/assets/javascripts/entrypoints/main_ee.js
deleted file mode 100644
index 5193bdd95c45e..0000000000000
--- a/app/assets/javascripts/entrypoints/main_ee.js
+++ /dev/null
@@ -1,5 +0,0 @@
-import { runModules } from '~/run_modules';
-
-const modules = import.meta.glob('../../../../ee/app/assets/javascripts/pages/**/index.js');
-
-runModules(modules, 'EE');
diff --git a/app/assets/javascripts/entrypoints/main_jh.js b/app/assets/javascripts/entrypoints/main_jh.js
deleted file mode 100644
index 8681ea933426e..0000000000000
--- a/app/assets/javascripts/entrypoints/main_jh.js
+++ /dev/null
@@ -1,5 +0,0 @@
-import { runModules } from '~/run_modules';
-
-const modules = import.meta.glob('../../../../jh/app/assets/javascripts/pages/**/index.js');
-
-runModules(modules, 'JH');
diff --git a/app/assets/javascripts/run_modules.js b/app/assets/javascripts/run_modules.js
deleted file mode 100644
index b68180781b380..0000000000000
--- a/app/assets/javascripts/run_modules.js
+++ /dev/null
@@ -1,48 +0,0 @@
-const paths = [];
-const allModules = {};
-
-const prefixes = {
- CE: '../pages/',
- EE: '../../../../ee/app/assets/javascripts/pages/',
- JH: '../../../../jh/app/assets/javascripts/pages/',
-};
-
-const editionExcludes = {
- JH: [],
- EE: [prefixes.JH],
- CE: [prefixes.EE, prefixes.JH],
-};
-
-const runWithExcludes = (edition) => {
- const prefix = prefixes[edition];
- const excludes = editionExcludes[edition];
- paths.forEach((path) => {
- const hasDuplicateEntrypoint = excludes.some(
- (editionPrefix) => `${editionPrefix}${path}` in allModules,
- );
- if (!hasDuplicateEntrypoint) allModules[`${prefix}${path}`]?.();
- });
-};
-
-let pathsPopulated = false;
-
-const populatePaths = () => {
- if (pathsPopulated) return;
- paths.push(
- ...document
- .querySelector('meta[name="controller-path"]')
- .content.split('/')
- .map((part, index, arr) => `${[...arr.slice(0, index), part].join('/')}/index.js`),
- );
- pathsPopulated = true;
-};
-
-export const runModules = (modules, edition) => {
- populatePaths();
- Object.assign(allModules, modules);
- // wait before all modules have been collected to exclude duplicates between CE and EE\JH
- // <script> runs as a macrotask, can't schedule with promises here
- requestAnimationFrame(() => {
- runWithExcludes(edition);
- });
-};
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index adb576d22cdcd..2336dacc928f1 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -499,15 +499,6 @@ def hidden_resource_icon(resource, css_class: nil)
end
end
- def controller_full_path
- action = case controller.action_name
- when 'create' then 'new'
- when 'update' then 'edit'
- else controller.action_name
- end
- "#{controller.controller_path}/#{action}"
- end
-
private
def browser_id
diff --git a/app/helpers/vite_helper.rb b/app/helpers/vite_helper.rb
index 556155f3c8597..644fc65427063 100644
--- a/app/helpers/vite_helper.rb
+++ b/app/helpers/vite_helper.rb
@@ -15,4 +15,17 @@ def vite_hmr_websocket_url
def vite_hmr_http_url
ViteRuby.env['VITE_HMR_HTTP_URL']
end
+
+ def vite_page_entrypoint_paths
+ action = case controller.action_name
+ when 'create' then 'new'
+ when 'update' then 'edit'
+ else controller.action_name
+ end
+
+ parts = (controller.controller_path.split('/') << action)
+
+ parts.map
+ .with_index { |part, idx| "pages.#{(parts[0, idx] << part).join('.')}.js" }
+ end
end
diff --git a/app/views/layouts/_head.html.haml b/app/views/layouts/_head.html.haml
index 4be82f6339ad9..f6c71fd6fb199 100644
--- a/app/views/layouts/_head.html.haml
+++ b/app/views/layouts/_head.html.haml
@@ -55,14 +55,9 @@
= webpack_bundle_tag 'performance_bar' if performance_bar_enabled?
- if vite_enabled?
- %meta{ name: 'controller-path', content: controller_full_path }
- if Rails.env.development?
= vite_client_tag
- = vite_javascript_tag "main"
- - if Gitlab.ee?
- = vite_javascript_tag "main_ee"
- - if Gitlab.jh?
- = vite_javascript_tag "main_jh"
+ = vite_javascript_tag "main", *vite_page_entrypoint_paths
= yield :page_specific_javascripts
diff --git a/config/webpack.config.js b/config/webpack.config.js
index 52cca8fa6dee9..fda6bb7b5643f 100644
--- a/config/webpack.config.js
+++ b/config/webpack.config.js
@@ -22,7 +22,6 @@ const BABEL_VERSION = require('@babel/core/package.json').version;
const BABEL_LOADER_VERSION = require('babel-loader/package.json').version;
const CompressionPlugin = require('compression-webpack-plugin');
const CopyWebpackPlugin = require('copy-webpack-plugin');
-const glob = require('glob');
// eslint-disable-next-line import/no-dynamic-require
const { VueLoaderPlugin } = require(VUE_LOADER_MODULE);
// eslint-disable-next-line import/no-dynamic-require
@@ -45,6 +44,7 @@ const {
GITLAB_WEB_IDE_PUBLIC_PATH,
copyFilesPatterns,
} = require('./webpack.constants');
+const { generateEntries } = require('./webpack.helpers');
const createIncrementalWebpackCompiler = require('./helpers/incremental_webpack_compiler');
const vendorDllHash = require('./helpers/vendor_dll_hash');
@@ -90,10 +90,6 @@ if (WEBPACK_REPORT) {
const devtool = IS_PRODUCTION ? 'source-map' : 'cheap-module-eval-source-map';
-let autoEntriesCount = 0;
-let watchAutoEntries = [];
-const defaultEntries = ['./main'];
-
const incrementalCompiler = createIncrementalWebpackCompiler(
INCREMENTAL_COMPILER_RECORD_HISTORY,
INCREMENTAL_COMPILER_ENABLED,
@@ -101,82 +97,6 @@ const incrementalCompiler = createIncrementalWebpackCompiler(
INCREMENTAL_COMPILER_TTL,
);
-function generateEntries() {
- // generate automatic entry points
- const autoEntries = {};
- const autoEntriesMap = {};
- const pageEntries = glob.sync('pages/**/index.js', {
- cwd: path.join(ROOT_PATH, 'app/assets/javascripts'),
- });
- watchAutoEntries = [path.join(ROOT_PATH, 'app/assets/javascripts/pages/')];
-
- function generateAutoEntries(entryPath, prefix = '.') {
- const chunkPath = entryPath.replace(/\/index\.js$/, '');
- const chunkName = chunkPath.replace(/\//g, '.');
- autoEntriesMap[chunkName] = `${prefix}/${entryPath}`;
- }
-
- pageEntries.forEach((entryPath) => generateAutoEntries(entryPath));
-
- if (IS_EE) {
- const eePageEntries = glob.sync('pages/**/index.js', {
- cwd: path.join(ROOT_PATH, 'ee/app/assets/javascripts'),
- });
- eePageEntries.forEach((entryPath) => generateAutoEntries(entryPath, 'ee'));
- watchAutoEntries.push(path.join(ROOT_PATH, 'ee/app/assets/javascripts/pages/'));
- }
-
- if (IS_JH) {
- const eePageEntries = glob.sync('pages/**/index.js', {
- cwd: path.join(ROOT_PATH, 'jh/app/assets/javascripts'),
- });
- eePageEntries.forEach((entryPath) => generateAutoEntries(entryPath, 'jh'));
- watchAutoEntries.push(path.join(ROOT_PATH, 'jh/app/assets/javascripts/pages/'));
- }
-
- const autoEntryKeys = Object.keys(autoEntriesMap);
- autoEntriesCount = autoEntryKeys.length;
-
- // import ancestor entrypoints within their children
- autoEntryKeys.forEach((entry) => {
- const entryPaths = [autoEntriesMap[entry]];
- const segments = entry.split('.');
- while (segments.pop()) {
- const ancestor = segments.join('.');
- if (autoEntryKeys.includes(ancestor)) {
- entryPaths.unshift(autoEntriesMap[ancestor]);
- }
- }
- autoEntries[entry] = defaultEntries.concat(entryPaths);
- });
-
- /*
- If you create manual entries, ensure that these import `app/assets/javascripts/webpack.js` right at
- the top of the entry in order to ensure that the public path is correctly determined for loading
- assets async. See: https://webpack.js.org/configuration/output/#outputpublicpath
-
- Note: WebPack 5 has an 'auto' option for the public path which could allow us to remove this option
- Note 2: If you are using web-workers, you might need to reset the public path, see:
- https://gitlab.com/gitlab-org/gitlab/-/issues/321656
- */
-
- const manualEntries = {
- default: defaultEntries,
- legacy_sentry: './sentry/legacy_index.js',
- sentry: './sentry/index.js',
- performance_bar: './performance_bar/index.js',
- jira_connect_app: './jira_connect/subscriptions/index.js',
- sandboxed_mermaid: './lib/mermaid.js',
- redirect_listbox: './entrypoints/behaviors/redirect_listbox.js',
- sandboxed_swagger: './lib/swagger.js',
- super_sidebar: './entrypoints/super_sidebar.js',
- tracker: './entrypoints/tracker.js',
- analytics: './entrypoints/analytics.js',
- };
-
- return Object.assign(manualEntries, incrementalCompiler.filterEntryPoints(autoEntries));
-}
-
const alias = {
// Map Apollo client to apollo/client/core to prevent react related imports from being loaded
'@apollo/client$': '@apollo/client/core',
@@ -318,12 +238,42 @@ if (USE_VUE3) {
vueLoaderOptions.compiler = require.resolve('./vue3migration/compiler');
}
+const entriesState = {
+ autoEntriesCount: 0,
+ watchAutoEntries: [],
+};
+const defaultEntries = ['./main'];
+
module.exports = {
mode: IS_PRODUCTION ? 'production' : 'development',
context: path.join(ROOT_PATH, 'app/assets/javascripts'),
- entry: generateEntries,
+ entry: () => {
+ /*
+ If you create manual entries, ensure that these import `app/assets/javascripts/webpack.js` right at
+ the top of the entry in order to ensure that the public path is correctly determined for loading
+ assets async. See: https://webpack.js.org/configuration/output/#outputpublicpath
+
+ Note: WebPack 5 has an 'auto' option for the public path which could allow us to remove this option
+ Note 2: If you are using web-workers, you might need to reset the public path, see:
+ https://gitlab.com/gitlab-org/gitlab/-/issues/321656
+ */
+ return {
+ default: defaultEntries,
+ legacy_sentry: './sentry/legacy_index.js',
+ sentry: './sentry/index.js',
+ performance_bar: './performance_bar/index.js',
+ jira_connect_app: './jira_connect/subscriptions/index.js',
+ sandboxed_mermaid: './lib/mermaid.js',
+ redirect_listbox: './entrypoints/behaviors/redirect_listbox.js',
+ sandboxed_swagger: './lib/swagger.js',
+ super_sidebar: './entrypoints/super_sidebar.js',
+ tracker: './entrypoints/tracker.js',
+ analytics: './entrypoints/analytics.js',
+ ...incrementalCompiler.filterEntryPoints(generateEntries({ defaultEntries, entriesState })),
+ };
+ },
output: {
path: WEBPACK_OUTPUT_PATH,
@@ -522,7 +472,7 @@ module.exports = {
priority: 20,
name: 'main',
chunks: 'initial',
- minChunks: autoEntriesCount * 0.9,
+ minChunks: entriesState.autoEntriesCount * 0.9,
}),
prosemirror: {
priority: 17,
@@ -728,14 +678,16 @@ module.exports = {
if (hasMissingNodeModules) compilation.contextDependencies.add(nodeModulesPath);
// watch for changes to automatic entrypoints
- watchAutoEntries.forEach((watchPath) => compilation.contextDependencies.add(watchPath));
+ entriesState.watchAutoEntries.forEach((watchPath) =>
+ compilation.contextDependencies.add(watchPath),
+ );
// report our auto-generated bundle count
if (incrementalCompiler.enabled) {
- incrementalCompiler.logStatus(autoEntriesCount);
+ incrementalCompiler.logStatus(entriesState.autoEntriesCount);
} else {
console.log(
- `${autoEntriesCount} entries from '/pages' automatically added to webpack output.`,
+ `${entriesState.autoEntriesCount} entries from '/pages' automatically added to webpack output.`,
);
}
diff --git a/config/webpack.helpers.js b/config/webpack.helpers.js
new file mode 100644
index 0000000000000..dce1904b29853
--- /dev/null
+++ b/config/webpack.helpers.js
@@ -0,0 +1,69 @@
+const path = require('path');
+const glob = require('glob');
+const { IS_EE, IS_JH, ROOT_PATH } = require('./webpack.constants');
+
+function generateEntries({ defaultEntries, entriesState } = { defaultEntries: [] }) {
+ // generate automatic entry points
+ const autoEntries = {};
+ const autoEntriesMap = {};
+ const pageEntries = glob.sync('pages/**/index.js', {
+ cwd: path.join(ROOT_PATH, 'app/assets/javascripts'),
+ });
+ if (entriesState) {
+ Object.assign(entriesState, {
+ watchAutoEntries: [path.join(ROOT_PATH, 'app/assets/javascripts/pages/')],
+ });
+ }
+
+ function generateAutoEntries(entryPath, prefix = '.') {
+ const chunkPath = entryPath.replace(/\/index\.js$/, '');
+ const chunkName = chunkPath.replace(/\//g, '.');
+ autoEntriesMap[chunkName] = `${prefix}/${entryPath}`;
+ }
+
+ pageEntries.forEach((entryPath) => generateAutoEntries(entryPath));
+
+ if (IS_EE) {
+ const eePageEntries = glob.sync('pages/**/index.js', {
+ cwd: path.join(ROOT_PATH, 'ee/app/assets/javascripts'),
+ });
+ eePageEntries.forEach((entryPath) => generateAutoEntries(entryPath, 'ee'));
+ if (entriesState) {
+ entriesState.watchAutoEntries.push(path.join(ROOT_PATH, 'ee/app/assets/javascripts/pages/'));
+ }
+ }
+
+ if (IS_JH) {
+ const eePageEntries = glob.sync('pages/**/index.js', {
+ cwd: path.join(ROOT_PATH, 'jh/app/assets/javascripts'),
+ });
+ eePageEntries.forEach((entryPath) => generateAutoEntries(entryPath, 'jh'));
+ if (entriesState) {
+ entriesState.watchAutoEntries.push(path.join(ROOT_PATH, 'jh/app/assets/javascripts/pages/'));
+ }
+ }
+
+ const autoEntryKeys = Object.keys(autoEntriesMap);
+ if (entriesState) {
+ Object.assign(entriesState, {
+ autoEntriesCount: autoEntryKeys.length,
+ });
+ }
+
+ // import ancestor entrypoints within their children
+ autoEntryKeys.forEach((entry) => {
+ const entryPaths = [autoEntriesMap[entry]];
+ const segments = entry.split('.');
+ while (segments.pop()) {
+ const ancestor = segments.join('.');
+ if (autoEntryKeys.includes(ancestor)) {
+ entryPaths.unshift(autoEntriesMap[ancestor]);
+ }
+ }
+ autoEntries[entry] = defaultEntries.concat(entryPaths);
+ });
+
+ return autoEntries;
+}
+
+module.exports = { generateEntries };
diff --git a/package.json b/package.json
index 7143a2c965f5f..26af246c5dd1b 100644
--- a/package.json
+++ b/package.json
@@ -45,7 +45,8 @@
"swagger:validate": "swagger-cli validate",
"webpack": "NODE_OPTIONS=\"${NODE_OPTIONS:=--max-old-space-size=5120}\" webpack --config config/webpack.config.js",
"webpack-vendor": "NODE_OPTIONS=\"${NODE_OPTIONS:=--max-old-space-size=5120}\" webpack --config config/webpack.vendor.config.js",
- "webpack-prod": "NODE_OPTIONS=\"${NODE_OPTIONS:=--max-old-space-size=5120}\" NODE_ENV=production webpack --config config/webpack.config.js"
+ "webpack-prod": "NODE_OPTIONS=\"${NODE_OPTIONS:=--max-old-space-size=5120}\" NODE_ENV=production webpack --config config/webpack.config.js",
+ "vite-prod": "NODE_OPTIONS=\"${NODE_OPTIONS:=--max-old-space-size=8000}\" NODE_ENV=production vite build"
},
"dependencies": {
"@apollo/client": "^3.5.10",
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index 18d0e92372dc5..dc28ffe657e52 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -979,38 +979,4 @@ def stub_controller_method(method_name, value)
end
end
end
-
- describe '#controller_full_path' do
- let(:path) { 'some_path' }
- let(:action) { 'show' }
-
- before do
- allow(helper.controller).to receive(:controller_path).and_return(path)
- allow(helper.controller).to receive(:action_name).and_return(action)
- end
-
- context 'when is create action' do
- let(:action) { 'create' }
-
- it 'transforms to "new" path' do
- expect(helper.controller_full_path).to eq("#{path}/new")
- end
- end
-
- context 'when is update action' do
- let(:action) { 'update' }
-
- it 'transforms to "edit" path' do
- expect(helper.controller_full_path).to eq("#{path}/edit")
- end
- end
-
- context 'when is show action' do
- let(:action) { 'show' }
-
- it 'passes through' do
- expect(helper.controller_full_path).to eq("#{path}/#{action}")
- end
- end
- end
end
diff --git a/spec/helpers/vite_helper_spec.rb b/spec/helpers/vite_helper_spec.rb
new file mode 100644
index 0000000000000..fc3f017691f22
--- /dev/null
+++ b/spec/helpers/vite_helper_spec.rb
@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ViteHelper, feature_category: :tooling do
+ describe '#vite_page_entrypoint_path' do
+ using RSpec::Parameterized::TableSyntax
+
+ where(:path, :action, :result) do
+ 'some_path' | 'create' | %w[pages.some_path.js pages.some_path.new.js]
+ 'some_path' | 'new' | %w[pages.some_path.js pages.some_path.new.js]
+ 'some_path' | 'update' | %w[pages.some_path.js pages.some_path.edit.js]
+ 'some_path' | 'show' | %w[pages.some_path.js pages.some_path.show.js]
+ 'some/long' | 'path' | %w[pages.some.js pages.some.long.js pages.some.long.path.js]
+ end
+
+ with_them do
+ before do
+ allow(helper.controller).to receive(:controller_path).and_return(path)
+ allow(helper.controller).to receive(:action_name).and_return(action)
+ end
+
+ it { expect(helper.vite_page_entrypoint_paths).to eq(result) }
+ end
+ end
+end
diff --git a/vite.config.js b/vite.config.js
index f1d3342c71908..7a0e78ced0391 100644
--- a/vite.config.js
+++ b/vite.config.js
@@ -10,6 +10,7 @@ import chokidar from 'chokidar';
import globby from 'globby';
import { viteCommonjs } from '@originjs/vite-plugin-commonjs';
import webpackConfig from './config/webpack.config';
+import { generateEntries } from './config/webpack.helpers';
import {
IS_EE,
IS_JH,
@@ -43,6 +44,15 @@ const emptyComponent = path.resolve(javascriptsPath, 'vue_shared/components/empt
const [rubyPlugin, ...rest] = RubyPlugin();
+const comment = '/* this is a virtual module used by Vite, it exists only in dev mode */\n';
+
+const virtualEntrypoints = Object.entries(generateEntries()).reduce((acc, [entryName, imports]) => {
+ const modulePath = imports[imports.length - 1];
+ const importPath = modulePath.startsWith('./') ? `~/${modulePath.substring(2)}` : modulePath;
+ acc[`${entryName}.js`] = `${comment}/* ${modulePath} */ import '${importPath}';\n`;
+ return acc;
+}, {});
+
// We can't use regular 'resolve' which points to sourceCodeDir in vite.json
// Because we need for '~' alias to resolve to app/assets/javascripts
// We can't use javascripts folder in sourceCodeDir because we also need to resolve other assets
@@ -163,6 +173,22 @@ function viteCopyPlugin({ patterns }) {
};
}
+const entrypointsDir = '/javascripts/entrypoints/';
+const pageEntrypointsPlugin = {
+ name: 'page-entrypoints',
+ load(id) {
+ if (!id.startsWith('pages.')) {
+ return undefined;
+ }
+ return virtualEntrypoints[id] ?? `/* doesn't exist */`;
+ },
+ resolveId(source) {
+ const fixedSource = source.replace(entrypointsDir, '');
+ if (fixedSource.startsWith('pages.')) return { id: fixedSource };
+ return undefined;
+ },
+};
+
export default defineConfig({
cacheDir: path.resolve(__dirname, 'tmp/cache/vite'),
resolve: {
@@ -181,6 +207,7 @@ export default defineConfig({
],
},
plugins: [
+ pageEntrypointsPlugin,
viteCSSCompilerPlugin({ shouldWatch: viteGDKConfig.hmr !== null }),
viteTailwindCompilerPlugin({ shouldWatch: viteGDKConfig.hmr !== null }),
viteCopyPlugin({
@@ -238,4 +265,12 @@ export default defineConfig({
worker: {
format: 'es',
},
+ build: {
+ rollupOptions: {
+ input: Object.keys(virtualEntrypoints).reduce((acc, value) => {
+ acc[value] = value;
+ return acc;
+ }, {}),
+ },
+ },
});
--
GitLab