sadpandajoe commented on code in PR #44459:
URL: https://github.com/apache/superset/pull/44459#discussion_r4445066301


##########
superset-frontend/package.json:
##########
@@ -226,8 +226,7 @@
     "use-immer": "^0.11.0",
     "use-query-params": "^2.2.2",
     "uuid": "^14.0.3",
-    "xlsx": "https://cdn.sheetjs.com/xlsx-0.20.3/xlsx-0.20.3.tgz";,
-    "yargs": "^18.2.0"
+    "xlsx": "https://cdn.sheetjs.com/xlsx-0.20.3/xlsx-0.20.3.tgz";

Review Comment:
   `yargs` is dropped from the dependencies here, but `webpack.config.js` still 
has `import yargs from 'yargs'` / `import { hideBin } from 'yargs/helpers'` and 
`const parsedArgs = yargs(...)` (lines 36-37 and 53), and `parsedArgs` is no 
longer read anywhere. The import only keeps resolving because `lerna` (a 
devDependency) happens to hoist `yargs@18`. Could those two imports and the 
unused `parsedArgs` be removed from `webpack.config.js` as part of this change?



##########
superset-frontend/scripts/build.js:
##########
@@ -31,13 +31,27 @@
 
 import { spawnSync } from 'node:child_process';
 import { globSync } from 'node:fs';
-import yargs from 'yargs';
-import { hideBin } from 'yargs/helpers';
+import { parseArgs } from 'node:util';
 
 process.env.PATH = `./node_modules/.bin:${process.env.PATH}`;
 
-const { globs, skipLib } = yargs(hideBin(process.argv)).parse();
-const glob = globs?.length > 1 ? `{${globs.join(',')}}` : globs?.[0] || '*';
+const cliOptions = {
+  globs: {
+    type: 'string',
+    multiple: true,
+    default: ['*'],
+  },
+  skipLib: {

Review Comment:
   The option is declared as `skipLib`, but CI runs `npm run plugins:build -- 
--skip-lib` (`.github/workflows/superset-frontend.yml`). `parseArgs` does not 
camel-case, so that invocation now fails with an unknown-option error. The 
option also defaults to `true` where the old no-flag run built `lib/`, so 
`plugins:publish-all` and the release scripts (which prune `lib/` and then call 
this with no flags) would publish packages without their CommonJS output. 
Should this be `'skip-lib'` with `default: false`?



##########
superset-frontend/webpack.config.js:
##########
@@ -63,662 +63,671 @@ const ROOT_DIR = path.resolve(__dirname, '..');
 // any url prefix.
 const MINI_CSS_EXTRACT_PUBLICPATH = './';
 
-const {
-  mode = 'development',
-  devserverPort: cliPort,
-  devserverHost: cliHost,
-  measure = false,
-  nameChunks = false,
-} = parsedArgs;
-
-// Precedence: CLI args > env vars > defaults
-const devserverPort = cliPort || process.env.WEBPACK_DEVSERVER_PORT || 9000;
-const devserverHost =
-  cliHost || process.env.WEBPACK_DEVSERVER_HOST || '127.0.0.1';
-
-const isDevMode = mode !== 'production';
-const isDevServer = process.argv[1]?.includes('webpack-dev-server') ?? false;
-
-const defaultEntryFilename = isDevMode
-  ? '[name].[contenthash:8].entry.js'
-  : nameChunks
-    ? '[name].[chunkhash].entry.js'
-    : '[name].[chunkhash].entry.js';
-
-const defaultChunkFilename = isDevMode
-  ? '[name].[contenthash:8].chunk.js'
-  : nameChunks
-    ? '[name].[chunkhash].chunk.js'
-    : '[chunkhash].chunk.js';
-
-const output = {
-  path: BUILD_DIR,
-  publicPath: '/static/assets/',
-  filename: pathData =>
-    pathData.chunk?.name === 'service-worker'
-      ? '../service-worker.js'
-      : defaultEntryFilename,
-  chunkFilename: pathData =>
-    pathData.chunk?.name === 'service-worker'
-      ? '../service-worker.js'
-      : defaultChunkFilename,
-  copy: [
-    'package.json',
-    { from: 'src/assets/images', to: 'images' },
-    // maplibre-gl 6's ESM-only build loads its worker from a real URL
-    // instead of an inlined blob (see MapLibre.tsx for the matching
-    // maplibregl.setWorkerUrl() call). The worker's own bundle does a
-    // relative ESM import of its "shared" chunk, so both files must be
-    // copied verbatim, unhashed, into the same output directory.
-    {
-      from: 'node_modules/maplibre-gl/dist/maplibre-gl-worker.mjs',
-    },
-    {
-      from: 'node_modules/maplibre-gl/dist/maplibre-gl-shared.mjs',
-    },
-  ],
-};
-
-if (!isDevMode) {
-  output.clean = true;
-}
-
-const plugins = [
-  new webpack.ProvidePlugin({
-    process: 'process/browser.js',
-    ...(isDevMode ? { Buffer: ['buffer', 'Buffer'] } : {}), // Fix 
plugin-chart-paired-t-test broken Story
-  }),
-
-  // creates a manifest.json mapping of name to hashed output used in template 
files
-  new WebpackManifestPlugin({
-    publicPath: output.publicPath,
-    seed: { app: 'superset' },
-    // This enables us to include all relevant files for an entry
-    generate: (seed, files, entrypoints) => {
-      // Each entrypoint's chunk files in the format of
-      // {
-      //   entry: {
-      //     css: [],
-      //     js: []
-      //   }
-      // }
-      const entryFiles = {};
-      Object.entries(entrypoints).forEach(([entry, chunks]) => {
-        entryFiles[entry] = {
-          css: chunks
-            .filter(x => x.endsWith('.css'))
-            .map(x => `${output.publicPath}${x}`),
-          js: chunks
-            .filter(x => x.endsWith('.js') && x.match(/(?<!hot-update).js$/))
-            .map(x => `${output.publicPath}${x}`),
-        };
-      });
-      return {
-        ...seed,
-        entrypoints: entryFiles,
-      };
-    },
-    // Also write manifest.json to disk when running `npm run dev`.
-    // This is required for Flask to work.
-    writeToFileEmit: isDevMode && !isDevServer,
-  }),
-
-  // expose mode variable to other modules
-  new webpack.DefinePlugin({
-    'process.env.WEBPACK_MODE': JSON.stringify(mode),
-    'process.env.REDUX_DEFAULT_MIDDLEWARE':
-      process.env.REDUX_DEFAULT_MIDDLEWARE,
-    'process.env.SCARF_ANALYTICS': JSON.stringify(process.env.SCARF_ANALYTICS),
-  }),
-
-  // static pages
-  new HtmlWebpackPlugin({
-    template: './src/assets/staticPages/404.html',
-    inject: true,
-    chunks: [],
-    filename: '404.html',
-  }),
-  new HtmlWebpackPlugin({
-    template: './src/assets/staticPages/500.html',
-    inject: true,
-    chunks: [],
-    filename: '500.html',
-  }),
-  new ModuleFederationPlugin({
-    name: 'superset',
-    filename: 'remoteEntry.js',
-    shared: {
-      react: {
-        singleton: true,
-        eager: true,
-        requiredVersion: packageConfig.dependencies.react,
-      },
-      'react-dom': {
-        singleton: true,
-        eager: true,
-        requiredVersion: packageConfig.dependencies['react-dom'],
+const generateWebpackConfigWithCustomInputs = (env = {}, args = {}) => {
+  const {
+    devserverPort: cliPort,
+    devserverHost: cliHost,
+    measure = false,
+    nameChunks = false,
+  } = env;
+  const { mode = 'development' } = args;
+
+  // Precedence: CLI args > env vars > defaults
+  const devserverPort = cliPort || process.env.WEBPACK_DEVSERVER_PORT || 9000;
+  const devserverHost =
+    cliHost || process.env.WEBPACK_DEVSERVER_HOST || '127.0.0.1';
+
+  const isDevMode = mode !== 'production';
+  const isDevServer = process.argv[1]?.includes('webpack-dev-server') ?? false;
+
+  const defaultEntryFilename = isDevMode
+    ? '[name].[contenthash:8].entry.js'
+    : nameChunks
+      ? '[name].[chunkhash].entry.js'
+      : '[name].[chunkhash].entry.js';
+
+  const defaultChunkFilename = isDevMode
+    ? '[name].[contenthash:8].chunk.js'
+    : nameChunks
+      ? '[name].[chunkhash].chunk.js'
+      : '[chunkhash].chunk.js';
+
+  const output = {
+    path: BUILD_DIR,
+    publicPath: '/static/assets/',
+    filename: pathData =>
+      pathData.chunk?.name === 'service-worker'
+        ? '../service-worker.js'
+        : defaultEntryFilename,
+    chunkFilename: pathData =>
+      pathData.chunk?.name === 'service-worker'
+        ? '../service-worker.js'
+        : defaultChunkFilename,
+    copy: [
+      'package.json',
+      { from: 'src/assets/images', to: 'images' },
+      // maplibre-gl 6's ESM-only build loads its worker from a real URL
+      // instead of an inlined blob (see MapLibre.tsx for the matching
+      // maplibregl.setWorkerUrl() call). The worker's own bundle does a
+      // relative ESM import of its "shared" chunk, so both files must be
+      // copied verbatim, unhashed, into the same output directory.
+      {
+        from: 'node_modules/maplibre-gl/dist/maplibre-gl-worker.mjs',
       },
-      antd: {
-        singleton: true,
-        requiredVersion: packageConfig.dependencies.antd,
-        eager: true,
+      {
+        from: 'node_modules/maplibre-gl/dist/maplibre-gl-shared.mjs',
       },
-      '@apache-superset/core': {
-        singleton: true,
-        eager: true,
+    ],
+  };
+
+  if (!isDevMode) {
+    output.clean = true;
+  }
+
+  const plugins = [
+    new webpack.ProvidePlugin({
+      process: 'process/browser.js',
+      ...(isDevMode ? { Buffer: ['buffer', 'Buffer'] } : {}), // Fix 
plugin-chart-paired-t-test broken Story
+    }),
+
+    // creates a manifest.json mapping of name to hashed output used in 
template files
+    new WebpackManifestPlugin({
+      publicPath: output.publicPath,
+      seed: { app: 'superset' },
+      // This enables us to include all relevant files for an entry
+      generate: (seed, files, entrypoints) => {
+        // Each entrypoint's chunk files in the format of
+        // {
+        //   entry: {
+        //     css: [],
+        //     js: []
+        //   }
+        // }
+        const entryFiles = {};
+        Object.entries(entrypoints).forEach(([entry, chunks]) => {
+          entryFiles[entry] = {
+            css: chunks
+              .filter(x => x.endsWith('.css'))
+              .map(x => `${output.publicPath}${x}`),
+            js: chunks
+              .filter(x => x.endsWith('.js') && x.match(/(?<!hot-update).js$/))
+              .map(x => `${output.publicPath}${x}`),
+          };
+        });
+        return {
+          ...seed,
+          entrypoints: entryFiles,
+        };
       },
-    },
-  }),
-];
-
-if (!process.env.CI) {
-  plugins.push(new webpack.ProgressPlugin());
-}
-
-// Add React Refresh plugin for development mode
-if (isDevMode) {
-  plugins.push(
-    new ReactRefreshWebpackPlugin({
-      // Exclude:
-      //   - node_modules (the plugin's default — must be re-added when 
overriding
-      //     `exclude`, otherwise pre-bundled ESM packages such as
-      //     react-checkbox-tree get the refresh loader injected into their
-      //     nested webpack runtime, causing
-      //     `__webpack_require__.$Refresh$ is undefined` at module factory
-      //     execution time.
-      //   - service worker (runs in a worker context without DOM/window and
-      //     does not need HMR).
-      exclude: [/node_modules/, /service-worker/],
+      // Also write manifest.json to disk when running `npm run dev`.
+      // This is required for Flask to work.
+      writeToFileEmit: isDevMode && !isDevServer,
     }),
-  );
-}
-
-if (!isDevMode) {
-  // CSS extraction for production builds
-  plugins.push(
-    new MiniCssExtractPlugin({
-      filename: '[name].[chunkhash].entry.css',
-      chunkFilename: '[name].[chunkhash].chunk.css',
+
+    // expose mode variable to other modules
+    new webpack.DefinePlugin({
+      'process.env.WEBPACK_MODE': JSON.stringify(mode),
+      'process.env.REDUX_DEFAULT_MIDDLEWARE':
+        process.env.REDUX_DEFAULT_MIDDLEWARE,
+      'process.env.SCARF_ANALYTICS': JSON.stringify(
+        process.env.SCARF_ANALYTICS,
+      ),
     }),
-  );
-}
-
-// Type checking does not run inside webpack: SWC only transpiles, and the
-// native TypeScript compiler is fast enough to run on its own. Use
-// `npm run type` (or `npm run type -- --watch`) for type errors, and
-// `npm run plugins:build` (or `npx tsc --build --watch`) to refresh the
-// packages' .d.ts output while editing plugins/packages.
-
-// In dev mode, include theme.ts in preamble to avoid separate chunk HMR issues
-const PREAMBLE = isDevMode
-  ? [path.join(APP_DIR, 'src/theme.ts'), path.join(APP_DIR, 'src/preamble.ts')]
-  : [path.join(APP_DIR, 'src/preamble.ts')];
-
-function addPreamble(entry) {
-  return PREAMBLE.concat([path.join(APP_DIR, entry)]);
-}
-
-// SWC configuration for TypeScript/JavaScript transpilation
-function createSwcLoader(syntax = 'typescript', tsx = true) {
-  return {
-    loader: 'swc-loader',
-    options: {
-      jsc: {
-        parser: {
-          syntax,
-          tsx: syntax === 'typescript' ? tsx : undefined,
-          jsx: syntax === 'ecmascript',
-          decorators: false,
-          dynamicImport: true,
+
+    // static pages
+    new HtmlWebpackPlugin({
+      template: './src/assets/staticPages/404.html',
+      inject: true,
+      chunks: [],
+      filename: '404.html',
+    }),
+    new HtmlWebpackPlugin({
+      template: './src/assets/staticPages/500.html',
+      inject: true,
+      chunks: [],
+      filename: '500.html',
+    }),
+    new ModuleFederationPlugin({
+      name: 'superset',
+      filename: 'remoteEntry.js',
+      shared: {
+        react: {
+          singleton: true,
+          eager: true,
+          requiredVersion: packageConfig.dependencies.react,
         },
-        transform: {
-          react: {
-            runtime: 'automatic',
-            importSource: '@emotion/react',
-            development: isDevMode,
-            refresh: isDevMode,
-          },
+        'react-dom': {
+          singleton: true,
+          eager: true,
+          requiredVersion: packageConfig.dependencies['react-dom'],
         },
-        target: 'es2015',
-        loose: true,
-        externalHelpers: false,
-        experimental: {
-          plugins: [
-            [
-              '@swc/plugin-emotion',
-              {
-                sourceMap: isDevMode,
-                autoLabel: isDevMode ? 'dev-only' : 'never',
-                labelFormat: '[local]',
-              },
-            ],
-            [
-              '@swc/plugin-transform-imports',
-              {
-                lodash: {
-                  transform: 'lodash/{{member}}',
-                  preventFullImport: true,
-                  skipDefaultConversion: false,
+        antd: {
+          singleton: true,
+          requiredVersion: packageConfig.dependencies.antd,
+          eager: true,
+        },
+        '@apache-superset/core': {
+          singleton: true,
+          eager: true,
+        },
+      },
+    }),
+  ];
+
+  if (!process.env.CI) {
+    plugins.push(new webpack.ProgressPlugin());
+  }
+
+  // Add React Refresh plugin for development mode
+  if (isDevMode) {
+    plugins.push(
+      new ReactRefreshWebpackPlugin({
+        // Exclude:
+        //   - node_modules (the plugin's default — must be re-added when 
overriding
+        //     `exclude`, otherwise pre-bundled ESM packages such as
+        //     react-checkbox-tree get the refresh loader injected into their
+        //     nested webpack runtime, causing
+        //     `__webpack_require__.$Refresh$ is undefined` at module factory
+        //     execution time.
+        //   - service worker (runs in a worker context without DOM/window and
+        //     does not need HMR).
+        exclude: [/node_modules/, /service-worker/],
+      }),
+    );
+  }
+
+  if (!isDevMode) {
+    // CSS extraction for production builds
+    plugins.push(
+      new MiniCssExtractPlugin({
+        filename: '[name].[chunkhash].entry.css',
+        chunkFilename: '[name].[chunkhash].chunk.css',
+      }),
+    );
+  }
+
+  // Type checking does not run inside webpack: SWC only transpiles, and the
+  // native TypeScript compiler is fast enough to run on its own. Use
+  // `npm run type` (or `npm run type -- --watch`) for type errors, and
+  // `npm run plugins:build` (or `npx tsc --build --watch`) to refresh the
+  // packages' .d.ts output while editing plugins/packages.
+
+  // In dev mode, include theme.ts in preamble to avoid separate chunk HMR 
issues
+  const PREAMBLE = isDevMode
+    ? [
+        path.join(APP_DIR, 'src/theme.ts'),
+        path.join(APP_DIR, 'src/preamble.ts'),
+      ]
+    : [path.join(APP_DIR, 'src/preamble.ts')];
+
+  function addPreamble(entry) {
+    return PREAMBLE.concat([path.join(APP_DIR, entry)]);
+  }
+
+  // SWC configuration for TypeScript/JavaScript transpilation
+  function createSwcLoader(syntax = 'typescript', tsx = true) {
+    return {
+      loader: 'swc-loader',
+      options: {
+        jsc: {
+          parser: {
+            syntax,
+            tsx: syntax === 'typescript' ? tsx : undefined,
+            jsx: syntax === 'ecmascript',
+            decorators: false,
+            dynamicImport: true,
+          },
+          transform: {
+            react: {
+              runtime: 'automatic',
+              importSource: '@emotion/react',
+              development: isDevMode,
+              refresh: isDevMode,
+            },
+          },
+          target: 'es2015',
+          loose: true,
+          externalHelpers: false,
+          experimental: {
+            plugins: [
+              [
+                '@swc/plugin-emotion',
+                {
+                  sourceMap: isDevMode,
+                  autoLabel: isDevMode ? 'dev-only' : 'never',
+                  labelFormat: '[local]',
                 },
-                'lodash-es': {
-                  transform: 'lodash-es/{{member}}',
-                  preventFullImport: true,
-                  skipDefaultConversion: false,
+              ],
+              [
+                '@swc/plugin-transform-imports',
+                {
+                  lodash: {
+                    transform: 'lodash/{{member}}',
+                    preventFullImport: true,
+                    skipDefaultConversion: false,
+                  },
+                  'lodash-es': {
+                    transform: 'lodash-es/{{member}}',
+                    preventFullImport: true,
+                    skipDefaultConversion: false,
+                  },
                 },
-              },
+              ],
             ],
-          ],
+          },
+        },
+        module: {
+          type: 'es6',
         },
       },
-      module: {
-        type: 'es6',
-      },
+    };
+  }
+
+  const config = {
+    entry: {
+      preamble: PREAMBLE,
+      // In dev mode, theme is included in preamble to avoid separate chunk 
HMR issues
+      ...(isDevMode ? {} : { theme: path.join(APP_DIR, 'src/theme.ts') }),
+      menu: addPreamble('src/views/menu.tsx'),
+      spa: addPreamble('src/views/index.tsx'),
+      embedded: addPreamble('src/embedded/index.tsx'),
+      'service-worker': path.join(APP_DIR, 'src/service-worker.ts'),
     },
-  };
-}
-
-const config = {
-  entry: {
-    preamble: PREAMBLE,
-    // In dev mode, theme is included in preamble to avoid separate chunk HMR 
issues
-    ...(isDevMode ? {} : { theme: path.join(APP_DIR, 'src/theme.ts') }),
-    menu: addPreamble('src/views/menu.tsx'),
-    spa: addPreamble('src/views/index.tsx'),
-    embedded: addPreamble('src/embedded/index.tsx'),
-    'service-worker': path.join(APP_DIR, 'src/service-worker.ts'),
-  },
-  cache: {
-    type: 'filesystem',
-    cacheDirectory: path.resolve(__dirname, '.temp_cache'),
-    // Separate cache for dev vs prod builds
-    name: `${isDevMode ? 'development' : 'production'}-cache`,
-    // Invalidate cache when these files change
-    buildDependencies: {
-      config: [
-        __filename,
-        path.resolve(__dirname, 'package-lock.json'),
-        path.resolve(__dirname, 'babel.config.js'),
-        path.resolve(__dirname, 'tsconfig.json'),
-      ],
+    cache: {
+      type: 'filesystem',
+      cacheDirectory: path.resolve(__dirname, '.temp_cache'),
+      // Separate cache for dev vs prod builds
+      name: `${isDevMode ? 'development' : 'production'}-cache`,
+      // Invalidate cache when these files change
+      buildDependencies: {
+        config: [
+          __filename,
+          path.resolve(__dirname, 'package-lock.json'),
+          path.resolve(__dirname, 'babel.config.js'),
+          path.resolve(__dirname, 'tsconfig.json'),
+        ],
+      },
+      // Compress cache for smaller disk usage (slight CPU tradeoff)
+      compression: isDevMode ? false : 'gzip',
     },
-    // Compress cache for smaller disk usage (slight CPU tradeoff)
-    compression: isDevMode ? false : 'gzip',
-  },
-  output,
-  stats: 'minimal',
-  /*
+    output,
+    stats: 'minimal',
+    /*
    Silence warning for missing export in @data-ui's internal structure. This
    issue arises from an internal implementation detail of @data-ui. As it's
    non-critical, we suppress it to prevent unnecessary clutter in the build
    output. For more context, refer to:
    https://github.com/williaster/data-ui/issues/208#issuecomment-946966712
    */
-  ignoreWarnings: [
-    {
-      message:
-        /export 'withTooltipPropTypes' \(imported as 'vxTooltipPropTypes'\) 
was not found/,
-    },
-    {
-      message: /Can't resolve.*superset_text/,
-    },
-  ],
-  performance: {
-    assetFilter(assetFilename) {
-      // don't throw size limit warning on geojson and font files
-      return !/\.(map|geojson|woff2)$/.test(assetFilename);
-    },
-  },
-  optimization: {
-    sideEffects: true,
-    splitChunks: {
-      chunks: 'all',
-      // increase minSize for devMode to 1000kb because of sourcemap
-      minSize: isDevMode ? 1000000 : 20000,
-      name: nameChunks,
-      automaticNameDelimiter: '-',
-      minChunks: 2,
-      cacheGroups: {
-        automaticNamePrefix: 'chunk',
-        // basic stable dependencies
-        vendors: {
-          priority: 50,
-          name: 'vendors',
-          test: new RegExp(
-            `/node_modules/(${[
-              'react',
-              'react-dom',
-              'redux',
-              'react-redux',
-              'react-table',
-              'react-ace',
-              'webpack.*',
-              '@?babel.*',
-              'lodash.*',
-              'antd',
-              '@ant-design.*',
-              '.*bootstrap',
-              'moment',
-              'jquery',
-              'core-js.*',
-              '@emotion.*',
-              'd3',
-              
'd3-(array|color|scale|interpolate|format|selection|collection|time|time-format)',
-            ].join('|')})/`,
-          ),
-        },
-        // viz thumbnails are used in `addSlice` and `explore` page
-        thumbnail: {
-          name: 'thumbnail',
-          test: /thumbnail(Large)?\.(png|jpg)/i,
-          priority: 20,
-          enforce: true,
-        },
+    ignoreWarnings: [
+      {
+        message:
+          /export 'withTooltipPropTypes' \(imported as 'vxTooltipPropTypes'\) 
was not found/,
+      },
+      {
+        message: /Can't resolve.*superset_text/,
+      },
+    ],
+    performance: {
+      assetFilter(assetFilename) {
+        // don't throw size limit warning on geojson and font files
+        return !/\.(map|geojson|woff2)$/.test(assetFilename);
       },
     },
-    usedExports: 'global',
-    minimizer: [
-      new MinimizerPlugin({
-        test: /\.css(\?.*)?$/i,
-        minify: MinimizerPlugin.lightningCssMinify,
-        minimizerOptions: {
-          targets: LightningCSS.browserslistToTargets(
-            packageConfig.browserslist,
-          ),
-        },
-      }),
-      new MinimizerPlugin({
-        minify: MinimizerPlugin.swcMinify,
-        minimizerOptions: {
-          compress: {
-            drop_console: false,
+    optimization: {
+      sideEffects: true,
+      splitChunks: {
+        chunks: 'all',
+        // increase minSize for devMode to 1000kb because of sourcemap
+        minSize: isDevMode ? 1000000 : 20000,
+        name: nameChunks,
+        automaticNameDelimiter: '-',
+        minChunks: 2,
+        cacheGroups: {
+          automaticNamePrefix: 'chunk',
+          // basic stable dependencies
+          vendors: {
+            priority: 50,
+            name: 'vendors',
+            test: new RegExp(
+              `/node_modules/(${[
+                'react',
+                'react-dom',
+                'redux',
+                'react-redux',
+                'react-table',
+                'react-ace',
+                'webpack.*',
+                '@?babel.*',
+                'lodash.*',
+                'antd',
+                '@ant-design.*',
+                '.*bootstrap',
+                'moment',
+                'jquery',
+                'core-js.*',
+                '@emotion.*',
+                'd3',
+                
'd3-(array|color|scale|interpolate|format|selection|collection|time|time-format)',
+              ].join('|')})/`,
+            ),
           },
-          mangle: true,
-          format: {
-            comments: false,
+          // viz thumbnails are used in `addSlice` and `explore` page
+          thumbnail: {
+            name: 'thumbnail',
+            test: /thumbnail(Large)?\.(png|jpg)/i,
+            priority: 20,
+            enforce: true,
           },
         },
-      }),
-    ],
-  },
-  resolve: {
-    // resolve modules from `/superset_frontend/node_modules` and 
`/superset_frontend`
-    modules: [
-      'node_modules',
-      APP_DIR,
-      path.resolve(APP_DIR, 'packages'),
-      path.resolve(APP_DIR, 'plugins'),
-    ],
-    alias: {
-      '@storybook-shared': path.resolve(APP_DIR, '.storybook/shared'),
-      react: path.resolve(path.join(APP_DIR, './node_modules/react')),
-      // TODO: remove Handlebars alias once Handlebars NPM package has been 
updated to
-      // correctly support webpack import 
(https://github.com/handlebars-lang/handlebars.js/issues/953)
-      handlebars: 'handlebars/dist/handlebars.js',
-      /*
+      },
+      usedExports: 'global',
+      minimizer: [
+        new MinimizerPlugin({
+          test: /\.css(\?.*)?$/i,
+          minify: MinimizerPlugin.lightningCssMinify,
+          minimizerOptions: {
+            targets: LightningCSS.browserslistToTargets(
+              packageConfig.browserslist,
+            ),
+          },
+        }),
+        new MinimizerPlugin({
+          minify: MinimizerPlugin.swcMinify,
+          minimizerOptions: {
+            compress: {
+              drop_console: false,
+            },
+            mangle: true,
+            format: {
+              comments: false,
+            },
+          },
+        }),
+      ],
+    },
+    resolve: {
+      // resolve modules from `/superset_frontend/node_modules` and 
`/superset_frontend`
+      modules: [
+        'node_modules',
+        APP_DIR,
+        path.resolve(APP_DIR, 'packages'),
+        path.resolve(APP_DIR, 'plugins'),
+      ],
+      alias: {
+        '@storybook-shared': path.resolve(APP_DIR, '.storybook/shared'),
+        react: path.resolve(path.join(APP_DIR, './node_modules/react')),
+        // TODO: remove Handlebars alias once Handlebars NPM package has been 
updated to
+        // correctly support webpack import 
(https://github.com/handlebars-lang/handlebars.js/issues/953)
+        handlebars: 'handlebars/dist/handlebars.js',
+        /*
       Temporary workaround to prevent Webpack from resolving moment locale
       files, which are unnecessary for this project and causing build warnings.
       This prevents "Module not found" errors for moment locale files.
       */
-      'moment/min/moment-with-locales': false,
-    },
-    extensions: ['.ts', '.tsx', '.js', '.jsx', '.yml'],
-    fallback: {
-      fs: false,
-      vm: false,
-      path: false,
-      stream: resolveEsmModule('stream-browserify'),
-      ...(isDevMode ? { buffer: resolveEsmModule('buffer/') } : {}), // Fix 
plugin-chart-paired-t-test broken Story
+        'moment/min/moment-with-locales': false,
+      },
+      extensions: ['.ts', '.tsx', '.js', '.jsx', '.yml'],
+      fallback: {
+        fs: false,
+        vm: false,
+        path: false,
+        stream: resolveEsmModule('stream-browserify'),
+        ...(isDevMode ? { buffer: resolveEsmModule('buffer/') } : {}), // Fix 
plugin-chart-paired-t-test broken Story
+      },
     },
-  },
-  context: APP_DIR, // to automatically find tsconfig.json
-  module: {
-    rules: [
-      {
-        test: /datatables\.net.*/,
-        loader: 'imports-loader',
-        options: {
-          additionalCode: 'var define = false;',
+    context: APP_DIR, // to automatically find tsconfig.json
+    module: {
+      rules: [
+        {
+          test: /datatables\.net.*/,
+          loader: 'imports-loader',
+          options: {
+            additionalCode: 'var define = false;',
+          },
         },
-      },
-      {
-        test: /node_modules\/(@deck\.gl|@luma\.gl).*\.js$/,
-        loader: 'imports-loader',
-        options: {
-          additionalCode: 'var module = module || {exports: {}};',
+        {
+          test: /node_modules\/(@deck\.gl|@luma\.gl).*\.js$/,
+          loader: 'imports-loader',
+          options: {
+            additionalCode: 'var module = module || {exports: {}};',
+          },
         },
-      },
-      {
-        test: 
/node_modules\/(geostyler|geostyler-openlayers-parser|geostyler-mapbox-parser|geostyler-sld-parser)\/.*\.js$/,
-        resolve: {
-          fullySpecified: false,
+        {
+          test: 
/node_modules\/(geostyler|geostyler-openlayers-parser|geostyler-mapbox-parser|geostyler-sld-parser)\/.*\.js$/,
+          resolve: {
+            fullySpecified: false,
+          },
         },
-      },
-      {
-        test: /\.tsx?$/,
-        exclude: [/\.test.tsx?$/, /node_modules/],
-        use: [createSwcLoader('typescript', true)],
-      },
-      {
-        test: /\.jsx?$/,
-        // include source code for plugins, but exclude node_modules and test 
files within them
-        exclude: [/superset-ui.*\/node_modules\//, /\.test.jsx?$/],
-        include: [
-          new RegExp(`${APP_DIR}/(src|.storybook|plugins|packages)`),
-          ...['./src', './.storybook', './plugins', './packages'].map(p =>
-            path.resolve(__dirname, p),
-          ), // redundant but required for windows
-          /@encodable/,
-        ],
-        use: [createSwcLoader('ecmascript')],
-      },
-      {
-        test: /ace-builds.*\/worker-.*$/,
-        type: 'asset/resource',
-      },
-      {
-        test: /\.css$/,
-        include: [APP_DIR, /superset-ui.+\/src/],
-        use: [
-          isDevMode
-            ? 'style-loader'
-            : {
-                loader: MiniCssExtractPlugin.loader,
-                options: {
-                  publicPath: MINI_CSS_EXTRACT_PUBLICPATH,
+        {
+          test: /\.tsx?$/,
+          exclude: [/\.test.tsx?$/, /node_modules/],
+          use: [createSwcLoader('typescript', true)],
+        },
+        {
+          test: /\.jsx?$/,
+          // include source code for plugins, but exclude node_modules and 
test files within them
+          exclude: [/superset-ui.*\/node_modules\//, /\.test.jsx?$/],
+          include: [
+            new RegExp(`${APP_DIR}/(src|.storybook|plugins|packages)`),
+            ...['./src', './.storybook', './plugins', './packages'].map(p =>
+              path.resolve(__dirname, p),
+            ), // redundant but required for windows
+            /@encodable/,
+          ],
+          use: [createSwcLoader('ecmascript')],
+        },
+        {
+          test: /ace-builds.*\/worker-.*$/,
+          type: 'asset/resource',
+        },
+        {
+          test: /\.css$/,
+          include: [APP_DIR, /superset-ui.+\/src/],
+          use: [
+            isDevMode
+              ? 'style-loader'
+              : {
+                  loader: MiniCssExtractPlugin.loader,
+                  options: {
+                    publicPath: MINI_CSS_EXTRACT_PUBLICPATH,
+                  },
                 },
+            {
+              loader: 'css-loader',
+              options: {
+                sourceMap: !isDevMode,
               },
-          {
-            loader: 'css-loader',
-            options: {
-              sourceMap: !isDevMode,
             },
+          ],
+        },
+        /* for css linking images (and viz plugin thumbnails) */
+        {
+          test: /\.png$/,
+          issuer: {
+            not: [/\/src\/assets\/staticPages\//],
+          },
+          type: 'asset',
+          generator: {
+            filename: '[name].[contenthash:8][ext]',
           },
-        ],
-      },
-      /* for css linking images (and viz plugin thumbnails) */
-      {
-        test: /\.png$/,
-        issuer: {
-          not: [/\/src\/assets\/staticPages\//],
         },
-        type: 'asset',
-        generator: {
-          filename: '[name].[contenthash:8][ext]',
+        {
+          test: /\.png$/,
+          issuer: /\/src\/assets\/staticPages\//,
+          type: 'asset',
         },
-      },
-      {
-        test: /\.png$/,
-        issuer: /\/src\/assets\/staticPages\//,
-        type: 'asset',
-      },
-      {
-        test: /\.svg(\?v=\d+\.\d+\.\d+)?$/,
-        issuer: /\.([jt])sx?$/,
-        use: [
-          {
-            loader: '@svgr/webpack',
-            options: {
-              titleProp: true,
-              ref: true,
-              // this is the default value for the icon. Using other values
-              // here will replace width and height in svg with 1em
-              icon: false,
+        {
+          test: /\.svg(\?v=\d+\.\d+\.\d+)?$/,
+          issuer: /\.([jt])sx?$/,
+          use: [
+            {
+              loader: '@svgr/webpack',
+              options: {
+                titleProp: true,
+                ref: true,
+                // this is the default value for the icon. Using other values
+                // here will replace width and height in svg with 1em
+                icon: false,
+              },
             },
+          ],
+        },
+        {
+          test: /\.(jpg|gif)$/,
+          type: 'asset/resource',
+          generator: {
+            filename: '[name].[contenthash:8][ext]',
           },
-        ],
-      },
-      {
-        test: /\.(jpg|gif)$/,
-        type: 'asset/resource',
-        generator: {
-          filename: '[name].[contenthash:8][ext]',
         },
+        /* for font-awesome */
+        {
+          test: /\.(woff|woff2|eot|ttf|otf)$/i,
+          type: 'asset/resource',
+        },
+        {
+          test: /\.ya?ml$/,
+          include: ROOT_DIR,
+          loader: 'js-yaml-loader',
+        },
+        {
+          test: /\.geojson$/,
+          type: 'asset/resource',
+        },
+      ],
+    },
+    externals: {
+      cheerio: 'window',
+      'react/lib/ExecutionEnvironment': true,
+      'react/lib/ReactContext': true,
+    },
+    plugins,
+    devtool: isDevMode ? 'eval-cheap-module-source-map' : false,
+    watchOptions: isDevMode
+      ? {
+          // Watch all plugin and package source directories
+          ignored: [
+            '**/node_modules',
+            '**/.git',
+            '**/lib',
+            '**/esm',
+            '**/dist',
+            '**/.temp_cache',
+            '**/coverage',
+            '**/*.test.*',
+            '**/*.stories.*',
+            '**/*.geojson',
+          ],
+          // Poll-based watching is needed in Docker/VM where native fs events
+          // don't propagate from host to container.
+          poll: 2000,
+          // Aggregate changes before rebuilding
+          aggregateTimeout: 500,
+        }
+      : undefined,
+  };
+
+  // find all the symlinked plugins and use their source code for imports
+  Object.entries(packageConfig.dependencies).forEach(([pkg, relativeDir]) => {
+    const srcPath = path.join(APP_DIR, `./node_modules/${pkg}/src`);
+    const dir = relativeDir.replace('file:', '');
+
+    if (
+      (pkg.startsWith('@superset-ui') || pkg.startsWith('@apache-superset')) &&
+      fs.existsSync(srcPath)
+    ) {
+      console.log(`[Superset Plugin] Use symlink source for ${pkg} @ ${dir}`);
+      config.resolve.alias[pkg] = path.resolve(APP_DIR, `${dir}/src`);
+    }
+  });
+  console.log(''); // pure cosmetic new line
+
+  if (isDevMode) {
+    let proxyConfig = getProxyConfig();
+    // Set up a plugin to handle manifest updates
+    config.plugins = config.plugins || [];
+    config.plugins.push({
+      apply: compiler => {
+        const { afterEmit } = getCompilerHooks(compiler);
+        afterEmit.tap('ManifestPlugin', manifest => {
+          proxyConfig = getProxyConfig(manifest);
+        });
       },
-      /* for font-awesome */
-      {
-        test: /\.(woff|woff2|eot|ttf|otf)$/i,
-        type: 'asset/resource',
+    });
+
+    config.devServer = {
+      devMiddleware: {
+        publicPath: '/static/assets/',
+        writeToDisk: true,
       },
-      {
-        test: /\.ya?ml$/,
-        include: ROOT_DIR,
-        loader: 'js-yaml-loader',
+      historyApiFallback: true,
+      hot: 'only', // HMR only, no page reload fallback
+      liveReload: false,
+      host: devserverHost,
+      port: devserverPort,
+      allowedHosts: [
+        ...new Set([
+          devserverHost,
+          'localhost',
+          '.localhost',
+          '127.0.0.1',
+          '::1',
+          '.local',
+        ]),
+      ],
+      proxy: [() => proxyConfig],
+      client: {
+        overlay: {
+          errors: true,
+          warnings: false,
+          runtimeErrors: error => !/ResizeObserver/.test(error.message),
+        },
+        logging: 'info', // Show HMR messages
+        webSocketURL: {
+          hostname: '0.0.0.0',
+          pathname: '/ws',
+          port: 0,
+        },
       },
-      {
-        test: /\.geojson$/,
-        type: 'asset/resource',
+      static: {
+        directory: path.join(process.cwd(), '../static/assets'),
       },
-    ],
-  },
-  externals: {
-    cheerio: 'window',
-    'react/lib/ExecutionEnvironment': true,
-    'react/lib/ReactContext': true,
-  },
-  plugins,
-  devtool: isDevMode ? 'eval-cheap-module-source-map' : false,
-  watchOptions: isDevMode
-    ? {
-        // Watch all plugin and package source directories
-        ignored: [
-          '**/node_modules',
-          '**/.git',
-          '**/lib',
-          '**/esm',
-          '**/dist',
-          '**/.temp_cache',
-          '**/coverage',
-          '**/*.test.*',
-          '**/*.stories.*',
-          '**/*.geojson',
-        ],
-        // Poll-based watching is needed in Docker/VM where native fs events
-        // don't propagate from host to container.
-        poll: 2000,
-        // Aggregate changes before rebuilding
-        aggregateTimeout: 500,
-      }
-    : undefined,
-};
+    };
+  }
 
-// find all the symlinked plugins and use their source code for imports
-Object.entries(packageConfig.dependencies).forEach(([pkg, relativeDir]) => {
-  const srcPath = path.join(APP_DIR, `./node_modules/${pkg}/src`);
-  const dir = relativeDir.replace('file:', '');
-
-  if (
-    (pkg.startsWith('@superset-ui') || pkg.startsWith('@apache-superset')) &&
-    fs.existsSync(srcPath)
-  ) {
-    console.log(`[Superset Plugin] Use symlink source for ${pkg} @ ${dir}`);
-    config.resolve.alias[pkg] = path.resolve(APP_DIR, `${dir}/src`);
+  // To
+  // e.g. npm run package-stats
+  if (process.env.BUNDLE_ANALYZER) {
+    config.plugins.push(new BundleAnalyzerPlugin({ analyzerMode: 'static' }));
+    config.plugins.push(
+      // this creates an HTML page with a sunburst diagram of dependencies.
+      // you'll find it at superset/static/stats/statistics.html
+      // note that the file is >100MB so it's in .gitignore
+      new Visualizer({
+        filename: path.join('..', 'stats', 'statistics.html'),
+        throwOnError: true,
+      }),
+    );
   }
-});
-console.log(''); // pure cosmetic new line
-
-if (isDevMode) {
-  let proxyConfig = getProxyConfig();
-  // Set up a plugin to handle manifest updates
-  config.plugins = config.plugins || [];
-  config.plugins.push({
-    apply: compiler => {
-      const { afterEmit } = getCompilerHooks(compiler);
-      afterEmit.tap('ManifestPlugin', manifest => {
-        proxyConfig = getProxyConfig(manifest);
-      });
-    },
+
+  // Speed measurement is disabled by default
+  // Pass flag --measure=true to enable
+  // e.g. npm run build -- --measure=true

Review Comment:
   With the config now taking `(env, args)`, `measure` only arrives via `--env 
measure`. `webpack-cli` does not know a `--measure` option, so following this 
comment (`npm run build -- --measure=true`) is rejected rather than enabling 
Speed Measure. Could the example be updated to the `--env measure` form?



##########
superset-frontend/webpack.proxy-config.js:
##########
@@ -185,7 +169,14 @@ async function processHTML(proxyResponse, response) {
   response.end(toDevHTML(Buffer.concat(chunks).toString()));
 }
 
-export default function getProxyConfig(newManifest) {
+export default function getProxyConfig(newManifest, env = {}) {
+  const parsedEnvArgs = { ...importTimeEnvVars, ...env };
+  const { supersetPort = 8088, supersetUrl = null } = parsedEnvArgs;

Review Comment:
   The new `--env supersetPort=8081` form still never reaches this function: 
`webpack.config.js` calls `getProxyConfig()` and `getProxyConfig(manifest)` 
(lines 646 and 653) without forwarding `env`, so only `process.env` is read. 
The dev server would still proxy to `localhost:8088`, and `supersetUrl` would 
be ignored. Could both calls pass `env` through?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to