From e3139e7167059518090e6c9d8ef4149f07d41b85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Gra=CC=88=C3=9Fl?= Date: Sun, 19 May 2024 15:24:33 +0200 Subject: [PATCH] WIP --- package-lock.json | 6 +++--- packages/config-utils/src/federated-modules.ts | 4 ++++ packages/config-utils/src/proxy.ts | 8 ++++++++ packages/config/src/bin/dev-script.ts | 6 ++++++ packages/config/src/bin/webpack.plugins.ts | 4 ++++ packages/config/src/lib/createConfig.ts | 2 ++ packages/config/src/lib/createPlugins.ts | 18 +++++++++--------- packages/config/src/lib/index.ts | 3 +++ 8 files changed, 39 insertions(+), 12 deletions(-) diff --git a/package-lock.json b/package-lock.json index 694856874..2c438d30d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -42528,7 +42528,7 @@ }, "packages/components": { "name": "@redhat-cloud-services/frontend-components", - "version": "4.2.7", + "version": "4.2.8", "license": "Apache-2.0", "dependencies": { "@patternfly/react-component-groups": "^5.0.0", @@ -42572,7 +42572,7 @@ }, "packages/config": { "name": "@redhat-cloud-services/frontend-components-config", - "version": "6.0.13", + "version": "6.0.14", "license": "Apache-2.0", "dependencies": { "@pmmmwh/react-refresh-webpack-plugin": "^0.5.8", @@ -43748,7 +43748,7 @@ }, "packages/tsc-transform-imports": { "name": "@redhat-cloud-services/tsc-transform-imports", - "version": "1.0.9", + "version": "1.0.10", "dependencies": { "glob": "10.3.3" }, diff --git a/packages/config-utils/src/federated-modules.ts b/packages/config-utils/src/federated-modules.ts index 96d044c26..bee9fb0f5 100644 --- a/packages/config-utils/src/federated-modules.ts +++ b/packages/config-utils/src/federated-modules.ts @@ -14,6 +14,10 @@ const createIncludes = (eager = false): { [module: string]: WebpackSharedConfig '@redhat-cloud-services/chrome': { singleton: true }, axios: {}, lodash: {}, + // TODO I would propose to remove this from the shared deps + // The benefit is minmal. redux promise middleware and similar packages that extend redux may be highly dependent + // on redux and different version can become incompatible and raise unexpected errors. + // We could also share redux and react-redux to minimise these issues, but apps could still add other similar packages. 'redux-promise-middleware': {}, react: { singleton: true, eager }, 'react-dom': { singleton: true, eager }, diff --git a/packages/config-utils/src/proxy.ts b/packages/config-utils/src/proxy.ts index 5b57188ad..359addfcd 100644 --- a/packages/config-utils/src/proxy.ts +++ b/packages/config-utils/src/proxy.ts @@ -136,6 +136,7 @@ const proxy = ({ routes, routesPath, useProxy, + // TODO It should be possible to set this as well from the outside. proxyURL = 'http://squid.corp.redhat.com:3128', standalone, port, @@ -223,6 +224,8 @@ const proxy = ({ } let standaloneConfig: ReturnType; + // TODO do we need this "standalone" mode still? + // We should at least extract this into it's own module if (standalone) { standaloneConfig = getConfig(standalone, localChrome, env, port); // Create network for services. @@ -362,6 +365,7 @@ const proxy = ({ ...(proxy.length > 0 && { proxy }), onListening(server) { if (useProxy || standaloneConfig) { + // TODO Refactor to use the hostname provided from a config/option higher up. const host = useProxy ? `${majorEnv}.foo.redhat.com` : 'localhost'; const origin = `http${server.options.https ? 's' : ''}://${host}:${server.options.port}`; fecLogger(LogType.info, ''); @@ -371,6 +375,7 @@ const proxy = ({ fecLogger(LogType.info, ''); } }, + // TODO Deprecated: needs to be replaced with `setupMiddleware` onBeforeSetupMiddleware({ app, compiler, options }) { app?.enable('strict routing'); // trailing slashes are mean @@ -383,6 +388,7 @@ const proxy = ({ * Allow serving chrome assets * This will allow running chrome as a host application */ + // TODO Most of this should be ...not here. if (!isChrome) { let chromePath = localChrome; if (standaloneConfig) { @@ -394,6 +400,7 @@ const proxy = ({ onBeforeSetupMiddleware({ chromePath }); + // TODO What is this? if (app && chromePath) { registerChrome({ app, @@ -405,6 +412,7 @@ const proxy = ({ } } + // TODO and this? registry.forEach((cb) => cb({ app, diff --git a/packages/config/src/bin/dev-script.ts b/packages/config/src/bin/dev-script.ts index 984c48e8e..7efe0ea18 100644 --- a/packages/config/src/bin/dev-script.ts +++ b/packages/config/src/bin/dev-script.ts @@ -124,6 +124,8 @@ async function devScript( process.exit(1); } } else { + // TODO This is neat, + // but we should just default to stage-stable, which is what we need in dev 99% of the time. await setEnv(cwd); } @@ -163,12 +165,16 @@ async function devScript( process.env.PROXY_VERBOSE = argv.verbose.toString(); } + // TODO Move to WebpackDevServer API instead of spawning a process + // Spawning a process does act weird when quitting the process, due to devservers way of quitting. + // It would also allow us to not have to (ab)use env variable to pass settings spawn(`npm exec -- webpack serve -c ${configPath}`, [], { stdio: [process.stdout, process.stdout, process.stdout], cwd, shell: true, }); + // TODO Move this to be middleware that won't need a separate process. if (fecConfig.interceptChromeConfig === true) { const interceptorServerPath = resolve(__dirname, './csc-interceptor-server.js'); const interceptorServerArgs = [interceptorServerPath]; diff --git a/packages/config/src/bin/webpack.plugins.ts b/packages/config/src/bin/webpack.plugins.ts index 5ac6773be..a7ecb62e6 100644 --- a/packages/config/src/bin/webpack.plugins.ts +++ b/packages/config/src/bin/webpack.plugins.ts @@ -36,6 +36,10 @@ const plugins = [ // Save 20kb of bundle size in prod if (process.env.NODE_ENV === 'production') { + // TODO This is questionable... + // The reason why redux logger would appear in prod is if it was mistakenly included. + // Replacing it with an empty module can, and has raised errors, that are especially hard to debug not knowing this bit here + // We should rather just be warning that there are dependencies that aren't recommended in prod plugins.push(new webpack.NormalModuleReplacementPlugin(/redux-logger/, resolve(__dirname, './empty.js'))); } diff --git a/packages/config/src/lib/createConfig.ts b/packages/config/src/lib/createConfig.ts index 21d46f199..cc6befe2b 100644 --- a/packages/config/src/lib/createConfig.ts +++ b/packages/config/src/lib/createConfig.ts @@ -261,6 +261,7 @@ export const createConfig = ({ directory: `${rootFolder || ''}/dist`, }, port: devServerPort, + // TODO deprecated and should be replaced with `server` when fully moving to webpack(devserver) v5 https: https || Boolean(useProxy), host: '0.0.0.0', // This shares on local network. Needed for docker.host.internal hot: internalHotReload, // Use livereload instead of HMR which is spotty with federated modules @@ -281,6 +282,7 @@ export const createConfig = ({ disableDotRule: true, }, devMiddleware: { + // TODO Figure out if this helps in any way or if it is required for something writeToDisk: true, }, client, diff --git a/packages/config/src/lib/createPlugins.ts b/packages/config/src/lib/createPlugins.ts index 5dc94a32c..6b4b8b429 100644 --- a/packages/config/src/lib/createPlugins.ts +++ b/packages/config/src/lib/createPlugins.ts @@ -52,15 +52,15 @@ export const createPlugins = ({ const hasTsConfig = glob.sync(path.resolve(rootFolder, './{tsconfig.json,!(node_modules)/**/tsconfig.json}')).length > 0; const fileHash = !internalHotReload && useFileHash; return [ - ...(generateSourceMaps - ? [ - new SourceMapDevToolPlugin({ - test: 'js', - exclude: /(node_modules|bower_components)/i, - filename: !fileHash ? 'sourcemaps/[name].js.map' : 'sourcemaps/[name].[contenthash].js.map', - }), - ] - : []), + // ...(generateSourceMaps + // ? [ + // new SourceMapDevToolPlugin({ + // test: 'js', + // exclude: /(node_modules|bower_components)/i, + // filename: !fileHash ? 'sourcemaps/[name].js.map' : 'sourcemaps/[name].[contenthash].js.map', + // }), + // ] + // : []), new MiniCssExtractPlugin({ chunkFilename: !fileHash ? 'css/[name].css' : 'css/[name].[contenthash].css', filename: !fileHash ? 'css/[name].css' : 'css/[name].[contenthash].css', diff --git a/packages/config/src/lib/index.ts b/packages/config/src/lib/index.ts index 9fb264917..e13b22202 100644 --- a/packages/config/src/lib/index.ts +++ b/packages/config/src/lib/index.ts @@ -53,9 +53,12 @@ const createFecConfig = ( config: ReturnType; plugins: ReturnType; } => { + // TODO ... sus. configurations.isProd = configurations.isProd || process.env.NODE_ENV === 'production'; const isProd = configurations.isProd; const { insights } = require(`${configurations.rootFolder}/package.json`); + + // TODO We should deprecated building based upon git branches let gitBranch; try { gitBranch = process.env.TRAVIS_BRANCH || process.env.BRANCH || gitRevisionPlugin.branch();