WIP

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 },

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<typeof getConfig>;
+  // 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,

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];

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')));
 }
 

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,

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',

packages/config/src/lib/index.ts

@@ -53,9 +53,12 @@ const createFecConfig = (
   config: ReturnType<typeof createConfig>;
   plugins: ReturnType<typeof createPlugins>;
 } => {
+  // 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();