refactor(fec): Fixes, improvements and moar options #2007

bastilian · 2024-05-18T19:43:33Z

This PR adds a (few) fix(es) and improvements to the fec command:

Enables npm run start commands for the config packages
 Just for convenience. I got tired of manually building all the time.
Fixes the port setting
 The —port flag was not fully working I thought that the PORT var was recognised by fec/webpack, but I realised it is something we have implemented in the apps’ fec config.
Adds an option to expose the “LOCAL_APPS” feature 
This is to make it more convenient to start an app adhoc with routes for another app, without having to use a ENV var. The code to build the routes has also been slightly refactored and now also allows to setup routes for /api as well via an --api option
Adds more cli flags for fec options
 These are flags for options that we have often configured in fec.config.js via ENV vars to allow them to be variable then calling fec. For example, in many apps we use a PROXY=true env variable to toggle useProxy int the fec config.
Rewrites downloading insights-chrome’s index.html 
This is to try and make the onBeforeSetupMiddleware obsolete, or at least reduce its responsibilities to make migrating to setupMiddleware easier or even unnecessary. It's also a cleaner implementation using the HTMLWebpackPlugin. At least a tiny bit. The cloning of the repo is removed as well. We really only need the index.html file, but if we realise we need more we can easily restore this.
Cleans up logging 
Where I touched code I replaced the console.log calls with fecLogger. Even webpack now logs via fecLogger. For consistency.
Adds an option to dump configs 
It has been very useful at times when debugging issues to inspect the fec config and webpack config.  This new option will make it more convenient without having to hack in some console.logs somewhere, and will show the full "fec" config passed in and the full resulting webpack config.

bastilian · 2024-05-19T14:19:07Z

packages/config-utils/src/fec-logger.ts

+  }
+
+  return {
+    ...console,


... it works, but I'm a bit unsure if that's right.

Hyperkid123 · 2024-05-20T07:24:04Z

packages/config-utils/src/fec-logger.ts

+  }
+
+  return {
+    ...console,


Hyperkid123 · 2024-05-20T07:29:38Z

packages/config/src/bin/dev.webpack.config.ts

  routes: internalProxyRoutes,
-  appUrl,
+  ...(fecConfig.appUrl ? { appUrl: createAppUrl(fecConfig.appUrl) } : {}),


If the app URL is not setup, the dev proxy won't really work. We should throw error rather than using fallback.

Hyperkid123 · 2024-05-20T07:30:47Z

packages/config-utils/src/federated-modules.ts

@@ -13,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


Yes, we should remove anything that is not a singleton. And generate the shared dependencies form that package.json.

With the exception to PF as we are generating the shared modules individually.

We should also avoid sharing anything without a version requirement.

Hyperkid123 · 2024-05-20T07:32:22Z

packages/config-utils/src/proxy.ts

-        process.exit();
+      if (!skipProxyCheck && hasCurlInstalled()) {
+        if (!checkLocalAppHost(appName, appUrl, appPort)) {
+          process.exit();


I'd add an exit code. We read it in other processes sometimes.

Hyperkid123 · 2024-05-20T07:34:07Z

packages/config-utils/src/proxy.ts

  }

  if (customProxy) {
    proxy.push(...customProxy);
  }

  let standaloneConfig: ReturnType<typeof getConfig>;
+  // TODO do we need this "standalone" mode still?
+  // We should at least extract this into it's own module


Some are using it, but I am not a fan of it. I don't think we can remove it just yet.

However, it was never maintained by us. At minimum I would deprecate the option and a log a warning.

Hyperkid123 · 2024-05-20T08:02:35Z

packages/config/src/lib/createConfig.ts

@@ -267,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


Used to be able to modify the build files in dev env.

Hyperkid123 · 2024-05-20T08:03:44Z

packages/config/src/lib/createConfig.ts

@@ -287,14 +303,12 @@ export const createConfig = ({
        proxyVerbose,
        target,
        registry,
-        onBeforeSetupMiddleware: ({ chromePath }) => {


This has to stay. Apps do not have their own HTML templates.

Hyperkid123 · 2024-05-20T08:04:39Z

packages/config/src/lib/createPlugins.ts

@@ -11,12 +12,19 @@ const ReactRefreshWebpackPlugin = require('@pmmmwh/react-refresh-webpack-plugin'
 const { glob } = require('glob');
 const path = require('path');

+const chromeIndexHtmlUrl = (env = 'dev-stable') => {


Can't use this. We will be removing the build repositories. Old build pipeline is coming to an end. We will be suing the container images as the source of truth for local chrome.

Hyperkid123 · 2024-05-20T08:05:18Z

packages/config/src/lib/fec.config.ts


 export interface FECConfiguration
  extends Partial<Omit<CreateConfigOptions, 'appUrl' | 'appName' | 'env' | 'rootFolder'>>,
    Partial<Omit<CreatePluginsOptions, 'rootFolder' | 'appName'>> {
  appName: never;
-  appUrl: string | string[] | (string | RegExp)[];
+  appUrl?: string | string[] | (string | RegExp)[];


This is mandatory.

Hyperkid123 · 2024-05-20T08:05:49Z

packages/config/src/lib/index.ts

  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


Yes. Remove only once all apps are on the current build pipeline.

bastilian force-pushed the fec_cli_fixes branch from 5c2b7c4 to 213d671 Compare May 18, 2024 21:22

bastilian changed the title ~~Fec cli fixes~~ refactor(fec): Fixes, improvements and moar options May 18, 2024

bastilian force-pushed the fec_cli_fixes branch from 213d671 to e3139e7 Compare May 19, 2024 13:24

bastilian commented May 19, 2024

View reviewed changes

Hyperkid123 reviewed May 20, 2024

View reviewed changes

bastilian mentioned this pull request Jun 21, 2024

Update documentation on how to run with local services RedHatInsights/insights-chrome#2871

Merged

aptmac mentioned this pull request Jun 25, 2024

feat(RHINENG-7074): Update inventory fec config RedHatInsights/insights-inventory-frontend#2194

Merged

WIP

b0e6e45

bastilian force-pushed the fec_cli_fixes branch from e3139e7 to b0e6e45 Compare August 30, 2024 00:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(fec): Fixes, improvements and moar options #2007

refactor(fec): Fixes, improvements and moar options #2007

bastilian commented May 18, 2024 •

edited

Loading

bastilian May 19, 2024

Hyperkid123 May 20, 2024

Hyperkid123 May 20, 2024

Hyperkid123 May 20, 2024

Hyperkid123 May 20, 2024

Hyperkid123 May 20, 2024

Hyperkid123 May 20, 2024

Hyperkid123 May 20, 2024

Hyperkid123 May 20, 2024

Hyperkid123 May 20, 2024

Hyperkid123 May 20, 2024

Hyperkid123 May 20, 2024

refactor(fec): Fixes, improvements and moar options #2007

Are you sure you want to change the base?

refactor(fec): Fixes, improvements and moar options #2007

Conversation

bastilian commented May 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bastilian commented May 18, 2024 •

edited

Loading