[CCI] Update webpack to v5 by andreymyssak · Pull Request #770 · opensearch-project/oui

andreymyssak · 2023-05-20T15:20:00Z

Description

Improved version of #765. It would take a long time to address all the potential improvements, so I created a new PR where I applied them.

Run the following commands again to see if there are any errors:

Issues Resolved

#584

Check List

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

andreymyssak · 2023-05-20T15:27:18Z

package.json

-    "@babel/plugin-proposal-class-properties": "^7.10.4",
-    "@babel/plugin-proposal-object-rest-spread": "^7.11.0",
-    "@babel/plugin-syntax-dynamic-import": "^7.8.3",
-    "@babel/plugin-transform-async-to-generator": "^7.10.4",


These plugins are included in @babel/preset-env

andreymyssak · 2023-05-20T15:31:14Z

package.json

-    "autoprefixer": "^9.8.6",
+    "autoprefixer": "^10.4.14",
    "axe-core": "^4.1.1",
-    "babel-core": "7.0.0-bridge.0",


Removed in favor of @babel/core

andreymyssak · 2023-05-20T15:31:43Z

package.json

-    "babel-plugin-inline-react-svg": "^1.1.1",
+    "babel-plugin-inline-react-svg": "^2.0.2",
    "babel-plugin-pegjs-inline-precompile": "^0.1.1",
-    "babel-template": "^6.26.0",


Removed in favor of @babel/template

andreymyssak · 2023-05-20T15:32:32Z

package.json

    "eslint-plugin-react": "^7.21.3",
    "eslint-plugin-react-hooks": "^4.1.2",
    "expose-gc": "^1.0.0",
-    "file-loader": "^6.1.0",


DEPRECATED for v5 (https://v4.webpack.js.org/loaders/file-loader/)

andreymyssak · 2023-05-20T15:34:03Z

package.json

    "prettier": "^2.1.2",
    "prop-types": "^15.6.0",
    "puppeteer": "^5.5.0",
-    "raw-loader": "^4.0.1",


DEPRECATED for v5 (https://v4.webpack.js.org/loaders/file-loader/). It was replaced with asset/source type (https://webpack.js.org/guides/asset-modules/#replacing-inline-loader-syntax)

andreymyssak · 2023-05-20T15:36:38Z

package.json

-    "webpack-dev-server": "^3.11.0",
+    "style-loader": "^3.3.2",
+    "terser-webpack-plugin": "^5.3.9",
+    "typescript": "4.1.6",


Typescript v4.0.5 gives an error below because @types/express-serve-static-core uses TypeScript v4.1 syntax, so I had to update it

node_modules/@types/express-serve-static-core/index.d.ts:104:68 - error TS1110: Type expected.

andreymyssak · 2023-05-20T15:39:47Z

scripts/babel/proptypes-from-ts-props/index.js

  const keysToRemove = new Set(
    toRemovePropTypes.arguments[0].elements
-      .map(keyToRemove =>
+      .map((keyToRemove) =>


prettier added parentheses, @BSFishy can you check if they were supposed to be added? If they shouldn't, I'll revert the prettier changes

What caused this change? I don't see any changes to anything related to Prettier. Is the file just not included in linting checks?

I don't see any issue with adding this change. It will keep our code style more consistent. I just want to know what caused it :)

I just enabled Prettier in the settings of my IDE, it seems that Prettier was not applied to some files in the script folder before, f.e.

scripts/babel/fetch-i18n-strings.js scripts/babel/react-docgen-typescript.js scripts/eslint-plugin/forward_ref_display_name.js scripts/eslint-plugin/i18n.js

and etc.

i am glad to see you are using a real IDE... unlike some people... 😉 at Ashwin.

andreymyssak · 2023-05-20T15:43:36Z

src/components/search_bar/search_filters.tsx

 import React, { Component, Fragment, ReactElement } from 'react';
 import { createFilter, SearchFilterConfig } from './filters';
-import { Query } from './query';
+import { Query } from './query/query';


Typescript doesn't want to export the Query class from the index.ts for reasons I don't understand, so I had to adjust the imports

andreymyssak · 2023-05-20T15:48:36Z

src/components/selectable/selectable_templates/selectable_template_sitewide.tsx

+      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+      // @ts-ignore


Typescript is great, but nowhere without pain 😭, I couldn't fix it, so I had to come up with a not-so-good solution - @ts-ignore

TS2322: Type '{ onFocus: (e: React.FocusEvent) => void; onBlur: (e: React.FocusEvent) => void; onInput: (e: React.FormEvent<...>) => void; ... 295 more ...; isPreFiltered?: boolean | undefined; }' is not assignable to type 'Partial<OuiSelectableSearchProps<{ className: string; prepend: {} | null | undefined; append: {} | null | undefined; icon?: OuiIconProps | undefined; avatar?: (Pick<HTMLAttributes, "children" | ... 251 more ... | "onTransitionEndCapture"> & CommonProps & DisambiguateSet<...> & { ...; } & { ...; }) | ...'. Types of property 'options' are incompatible. Type '((DisambiguateSet<OuiSelectableGroupLabelOption<{ [key: string]: any; }>, OuiSelectableLIOption<{ [key: string]: any; }>> & CommonProps & { ...; } & HTMLAttributes<...> & { ...; }) | (DisambiguateSet<...> & ... 3 more ... & { ...; }))[] | undefined' is not assignable to type '((DisambiguateSet<Pick<OuiSelectableOptionBase, "className" | "id" | "aria-label" | "data-test-subj" | "label" | "searchableLabel" | "key" | "checked" | "disabled" | "prepend" | "append" | "ref"> & HTMLAttributes<...> & { ...; } & { ...; }, CommonProps & ... 2 more ... & { ...; }> & CommonProps & { ...; } & HTMLAttr...'. Type '((DisambiguateSet<OuiSelectableGroupLabelOption<{ [key: string]: any; }>, OuiSelectableLIOption<{ [key: string]: any; }>> & CommonProps & { ...; } & HTMLAttributes<...> & { ...; }) | (DisambiguateSet<...> & ... 3 more ... & { ...; }))[]' is not assignable to type '((DisambiguateSet<Pick<OuiSelectableOptionBase, "className" | "id" | "aria-label" | "data-test-subj" | "label" | "searchableLabel" | "key" | "checked" | "disabled" | "prepend" | "append" | "ref"> & HTMLAttributes<...> & { ...; } & { ...; }, CommonProps & ... 2 more ... & { ...; }> & CommonProps & { ...; } & HTMLAttr...'. Type '(DisambiguateSet<OuiSelectableGroupLabelOption<{ [key: string]: any; }>, OuiSelectableLIOption<{ [key: string]: any; }>> & CommonProps & { ...; } & HTMLAttributes<...> & { ...; }) | (DisambiguateSet<...> & ... 3 more ... & { ...; })' is not assignable to type '(DisambiguateSet<Pick<OuiSelectableOptionBase, "className" | "id" | "aria-label" | "data-test-subj" | "label" | "searchableLabel" | "key" | "checked" | "disabled" | "prepend" | "append" | "ref"> & HTMLAttributes<...> & { ...; } & { ...; }, CommonProps & ... 2 more ... & { ...; }> & CommonProps & { ...; } & HTMLAttri...'. Type 'DisambiguateSet<OuiSelectableGroupLabelOption<{ [key: string]: any; }>, OuiSelectableLIOption<{ [key: string]: any; }>> & CommonProps & { ...; } & HTMLAttributes<...> & { ...; }' is not assignable to type '(DisambiguateSet<Pick<OuiSelectableOptionBase, "className" | "id" | "aria-label" | "data-test-subj" | "label" | "searchableLabel" | "key" | "checked" | "disabled" | "prepend" | "append" | "ref"> & HTMLAttributes<...> & { ...; } & { ...; }, CommonProps & ... 2 more ... & { ...; }> & CommonProps & { ...; } & HTMLAttri...'. Type 'DisambiguateSet<OuiSelectableGroupLabelOption<{ [key: string]: any; }>, OuiSelectableLIOption<{ [key: string]: any; }>> & CommonProps & { ...; } & HTMLAttributes<...> & { ...; }' is not assignable to type 'DisambiguateSet<Pick<OuiSelectableOptionBase, "className" | "id" | "aria-label" | "data-test-subj" | "label" | "searchableLabel" | "key" | "checked" | "disabled" | "prepend" | "append" | "ref"> & HTMLAttributes<...> & { ...; } & { ...; }, CommonProps & ... 2 more ... & { ...; }> & CommonProps & { ...; } & HTMLAttrib...'. Type 'DisambiguateSet<OuiSelectableGroupLabelOption<{ [key: string]: any; }>, OuiSelectableLIOption<{ [key: string]: any; }>> & CommonProps & { ...; } & HTMLAttributes<...> & { ...; }' is not assignable to type '{ className: string; prepend: {} | null | undefined; append: {} | null | undefined; icon?: OuiIconProps | undefined; avatar?: (Pick<React.HTMLAttributes, "children" | ... 251 more ... | "onTransitionEndCapture"> & CommonProps & DisambiguateSet<...> & { ...; } & { ...; }) | (Pick<...> & ... 4 more ......'. Types of property 'className' are incompatible. Type 'string | undefined' is not assignable to type 'string'. Type 'undefined' is not assignable to type 'string'.

We can create a followup issue to dive deeper into this

andreymyssak · 2023-05-20T15:59:01Z

package.json

-    "@babel/plugin-proposal-object-rest-spread": "^7.11.0",
-    "@babel/plugin-syntax-dynamic-import": "^7.8.3",
-    "@babel/plugin-transform-async-to-generator": "^7.10.4",
-    "@babel/plugin-transform-runtime": "^7.11.0",


It's not strictly necessary to use @babel/plugin-transform-runtime if we are already using core-js with useBuiltIns: 'usage' in @babel/preset-env

andreymyssak · 2023-05-21T07:50:18Z

src-docs/webpack.config.js

+const { NODE_ENV, CI, WEBPACK_SERVE } = process.env;

-const isDevelopment = WEBPACK_DEV_SERVER === 'true' && CI == null;
+const isDevelopment = WEBPACK_SERVE === 'true' && CI == null;


WEBPACK_DEV_SERVER was changed to WEBPACK_SERVE (webpack/webpack-cli#2027)

andreymyssak · 2023-05-21T07:54:10Z

src-docs/webpack.config.js

-function getPortSync(options) {
-  let isDone = false;
-  let freeport = null;
-  let error = null;
-
-  getPort(options)
-    .then((port) => {
-      isDone = true;
-      freeport = port;
-    })
-    .catch((err) => {
-      isDone = true;
-      error = err;
-    });
-
-  // wait until we're done'
-  deasync.loopWhile(() => !isDone);
-
-  if (error) {
-    throw error;
-  } else {
-    return freeport;
-  }
-}


getPort was not running for reasons I do not know, so I made webpack.config.js as a function and removed deasync

andreymyssak · 2023-05-21T08:38:15Z

.eslintrc.js

  plugins: ['jsx-a11y', 'prettier', 'local', 'react-hooks'],
  rules: {
    'prefer-template': 'error',
+    'import/no-unresolved': 'off',


Eslint started not passing due to the detection of errors like the following

68:30 error Unable to resolve path to module './search?raw' import/no-unresolved

This may be due to the fact that these files were indexed, but I'm not sure. If you go into the code locally in another branch, IDE will also show the same error. I haven't found a more elegant solution to fix this error, so I turned off the rule.

Is there an option to ignore query params? Maybe we could raise this to the ESLint team? Otherwise we could create a follow up issue to dive deeper into this

As it turned out, eslint-import-resolver-webpack only supports "synchronous" Webpack configs (https://github.com/import-js/eslint-plugin-import/tree/main/resolvers/webpack). How critical is it to use getPort? Is it possible to just put port 8030?

As it turned out, eslint-import-resolver-webpack only supports "synchronous" Webpack configs (https://github.com/import-js/eslint-plugin-import/tree/main/resolvers/webpack). How critical is it to use getPort? Is it possible to just put port 8030?

I'm pretty sure the reason for getPort is that you can actually have multiple instances of OUI running on different ports - you can replicate by doing yarn start in multiple terminal tabs/windows, and the port will auto increment without collisions.

BSFishy

Looks pretty good, just had a few questions. Running integration tests now to verify compatibility with Dashboards

BSFishy · 2023-05-22T16:46:55Z

.babelrc.js

+      {
+        // `targets` property set via `.browserslistrc`
+        useBuiltIns: process.env.NO_COREJS_POLYFILL ? false : 'usage',
+        corejs: 3,


In my testing, this line needed to be changed to

corejs: process.env.NO_COREJS_POLYFILL ? undefined : 3,

I think because it doesn't want the corejs version unless 'usage' is specified for useBuiltIns. Did you run into the same thing at all?

Just confirmed, a warning pops up when this line isn't changed:

WARNING (@babel/preset-env): The `corejs` option only has an effect when the `useBuiltIns` option is not `false`

corejs: process.env.NO_COREJS_POLYFILL ? undefined : 3,

I'm not sure about changing false to undefined because there is no difference between them. According to the documentation, the default value for useBuiltIns is false (https://babeljs.io/docs/babel-preset-env#usebuiltins). I guess it is worth leaving false because using values that are expected by the field will help avoid surprise issues in the future.

I think because it doesn't want the corejs version unless 'usage' is specified for useBuiltIns. Did you run into the same thing at all?

Yes, I get the same warning message. The reason for this is that we purposely do not include polyfill imports in our build (see https://github.com/opensearch-project/oui/blob/main/scripts/compile-oui.js#L190-L219). As far as I understand, this solution was inherited from here elastic/eui#1982.

Can any of you help me with why we need any polyfills?

I checked the code of babel/preset-env and @BSFishy's solution is a valid one.

BSFishy · 2023-05-22T16:50:22Z

scripts/babel/proptypes-from-ts-props/index.js

  const keysToRemove = new Set(
    toRemovePropTypes.arguments[0].elements
-      .map(keyToRemove =>
+      .map((keyToRemove) =>


What caused this change? I don't see any changes to anything related to Prettier. Is the file just not included in linting checks?

I don't see any issue with adding this change. It will keep our code style more consistent. I just want to know what caused it :)

BSFishy · 2023-05-22T16:57:30Z

src-docs/src/views/package/changelog.js


-const changelogSource = require('!!raw-loader!../../../../CHANGELOG.md').default.replace(
+const changelogSource = require('../../../../CHANGELOG.md?raw').replace(
  /## \[`master`\].+?##/s, // remove the `master` heading & contents


Just saw this, maybe since we renamed our default branch to main, we can update it here? Can create a followup issue for that.

BSFishy · 2023-05-22T17:03:29Z

src-docs/webpack.config.js

+          static: {
+            directory: path.resolve(__dirname, 'build'),
+            // prevent file watching while running on CI
+            // /app/ represents the entire docker environment
+            watch: isPuppeteer ? { ignored: '**/*' } : undefined,
+          },
          host: '0.0.0.0',
-        }),
-        disableHostCheck: true,
-        historyApiFallback: true,
-        // prevent file watching while running on CI
-        // /app/ represents the entire docker environment
-        watchOptions: isPuppeteer
-          ? {
-              ignored: '**/*',
-            }
-          : undefined,
-      }
-    : undefined,
-  node: {
-    fs: 'empty',
-  },
+          port,
+          allowedHosts: 'all',
+          historyApiFallback: true,


Why has the devServer setting changed so much? Did the API just change significantly? And do all the previous settings have an equivalent in the new settings?

the contentBase, ..., watchOptions were removed in favor of the static option (https://github.com/webpack/webpack-dev-server/blob/master/CHANGELOG.md#400-beta0-2020-11-27, https://github.com/webpack/webpack-dev-server/blob/master/CHANGELOG.md#static)

the disableHostCheck and allowedHosts options were removed in favor of the firewall option (https://github.com/webpack/webpack-dev-server/blob/master/CHANGELOG.md#400-beta0-2020-11-27)

rename firewall option to allowedHosts option (https://github.com/webpack/webpack-dev-server/blob/master/CHANGELOG.md#400-rc0-2021-07-19)

The other fields have not changed

BSFishy · 2023-05-22T17:04:29Z

src/components/icon/icon.test.tsx

        component.update();
        expect(prettyHtml(component.html())).toMatchSnapshot();
-        resolve();
+        resolve(null);


What prompted this change?

Updating Typescript to v4.1.6 prompted this change (see https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-1.html#resolves-parameters-are-no-longer-optional-in-promises), but I can use Promise<void> instead of resolve(null), thanks for catching this

BSFishy · 2023-05-22T17:05:27Z

src/components/search_bar/filters/field_value_selection_filter.tsx

 import { OuiSpacer } from '../../spacer';
 import { OuiIcon } from '../../icon';
-import { Query } from '../query';
+import { Query } from '../query/query';


Do we already export Query for ../query? If not, it seems like that might be desirable. Even more so for backwards compatibility reasons?

We have index.ts, but it doesn't work for class imports and that's weird

oui/src/components/search_bar/query/index.ts

Lines 31 to 32 in ce8acf4

export { Query } from './query';

export { AST } from './ast';

WARNING in ../../src/components/search_bar/search_bar.tsx 256:39-44 export 'Query' (imported as 'Query') was not found in './query' (possible exports: AST) @ ../../src/components/search_bar/index.ts 31:0-49 31:0-49 31:0-49 @ ../../src/components/index.js 94:0-49 94:0-49 94:0-49 @ ./routes.js 56:0-56 205:44-60 @ ./index.js 42:0-30 58:28-47

BSFishy · 2023-05-22T17:06:21Z

src/components/selectable/selectable_templates/selectable_template_sitewide.tsx

+      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+      // @ts-ignore


We can create a followup issue to dive deeper into this

BSFishy · 2023-05-22T17:07:57Z

src/webpack.config.js

  optimization: {
    minimize: isProduction,
    minimizer: [terserPlugin],
-    noEmitOnErrors: true,
+    emitOnErrors: true,
  },


In my testing, I had to add usedExports: false to disable some aggressive tree shaking. Did you run into the same thing at all?

In the webpack v5 migration, nothing was written about usedExports. If I'm not mistaken, it was also included by default for productions in version 4 (https://v4.webpack.js.org/configuration/optimization/#optimizationusedexports), so I'm not sure if we should turn it off.

BSFishy · 2023-05-22T17:10:46Z

.eslintrc.js

  plugins: ['jsx-a11y', 'prettier', 'local', 'react-hooks'],
  rules: {
    'prefer-template': 'error',
+    'import/no-unresolved': 'off',


Is there an option to ignore query params? Maybe we could raise this to the ESLint team? Otherwise we could create a follow up issue to dive deeper into this

BSFishy · 2023-05-22T17:28:10Z

Also 2086980 and e9a63af need to be reverted, as we want to backport this to 1.x :)

andreymyssak · 2023-05-29T14:35:25Z

Also 2086980 and e9a63af need to be reverted, as we want to backport this to 1.x :)

If I revet these commits, then my changes won't go into main. Should I create another PR where I just do the cherry-pick and the necessary changes for 1.x?

Co-authored-by: Sergey Myssak <sergey.myssak@gmail.com> Signed-off-by: Andrey Myssak <andreymyssak@gmail.com>

…ch-project#584) Co-authored-by: Sergey Myssak <sergey.myssak@gmail.com> Signed-off-by: Andrey Myssak <andreymyssak@gmail.com>

Co-authored-by: Sergey Myssak <sergey.myssak@gmail.com> Signed-off-by: Andrey Myssak <andreymyssak@gmail.com>

andreymyssak · 2023-07-13T17:18:45Z

Summary of Work Done

Dependencies

Bumped webpack from v4 to v5
Bumped postcss from v7 to v8
Bumped babel minor version
Bumped typescript from v4.0.5 to v4.1.6 ([CCI] Update webpack to v5 #770 (comment))
Removed some of the resolutions that were covered by a dependency update

Formatting

Prettier was not applied to some files in the script folder before, so the changes can be found in the following files (#770 (comment))

scripts/babel/fetch-i18n-strings.js
scripts/babel/react-docgen-typescript.js
scripts/eslint-plugin/forward_ref_display_name.js
scripts/eslint-plugin/i18n.js

Questions Requiring Further Discussion

Can we directly use port 8030 instead of utilizing getPortSync and getPort.makeRange?

During our migration to webpack v5, we ran into a problem with deasync, as it stopped working. Previously, it was used to convert our webpack.config.js from asynchronous to synchronous. However, since webpack supports asynchronous configurations, I've converted our config file accordingly. However, this change introduced a bottleneck.

As part of our adherence to best practices, the legacy !!raw-loader! syntax has been replaced with ?raw, such as:

const basicSource = require('!!raw-loader!./basic_window_event');
// is now
const basicSource = require('./basic_window_event?raw');

You can read more about this transition in the Webpack documentation.

However, we ran into a problem: eslint does not recognize resourceQuery. This is because eslint-import-resolver-webpack only supports synchronous webpack configurations (see here), while we are using an asynchronous one. Since deasync is no longer working in webpack.config.js, we can't go back to it.

To address this, we could potentially switch back to a synchronous configuration in our webpack.config.js. This could be accomplished by eliminating the use of getPortSync and using port 8030 directly. Would it be possible to simply use port 8030 instead of getPortSync and getPort.makeRange?

TypeScript refuses to export the Query class from index.ts for reasons that remain unclear. See the specific issue in the [code here].

The system throws the following warning message:

WARNING in ../../src/components/search_bar/search_bar.tsx 256:39-44
export 'Query' (imported as 'Query') was not found in './query' (possible exports: AST)
 @ ../../src/components/search_bar/index.ts 31:0-49 31:0-49 31:0-49
 @ ../../src/components/index.js 94:0-49 94:0-49 94:0-49
 @ ./routes.js 56:0-56 205:44-60
 @ ./index.js 42:0-30 58:28-47

For now, the issue has been resolved by importing the Query class from query/query.ts, instead of from query/index.ts.

There is a chance that a future update of the `TypeScript' version may have a fix for this problem.

Running into a complex type conflict with searchProps in OuiSelectable

We've encountered a type conflict problem with searchProps in the OuiSelectable component that is proving difficult to resolve. Details about the specific type error can be found here.

This problem is currently solved by using the @ts-ignore. There is a chance that a future update of the `TypeScript' version may have a fix for this problem.

Build process displays WARNING (@babel/preset-env): The corejs option only has an effect when the useBuiltIns option is not false.

Please note that the current warning does not cause a functional failure. For more details, please refer to the discussion in this thread.

Including usedExports in webpack.config.js. For more details, please refer to the discussion in this thread

AMoo-Miki · 2023-07-15T05:22:34Z

Thanks guys; this is a big deal and what you have done is awesome. I will pull this down and play with it a bit.

BSFishy · 2023-07-20T16:38:39Z

Thanks guys; this is a big deal and what you have done is awesome. I will pull this down and play with it a bit.

Additionally, make sure to test with Dashboards. I was running into issues where one of the auxiliary bundles wasn't exporting anything, causing issues in Dashboards. More info here: #765 (comment)

Signed-off-by: Josh Romero <rmerqg@amazon.com>

joshuarrrr · 2023-09-13T19:52:20Z

@andreymyssak I took a shot at resolving the conflicts, but take a look at 22fb706 to see if they make sense.

BSFishy · 2023-09-18T17:13:13Z

Thinking about this more, could it be possible that the sass-vars-to-js-loader loader is using some sort of Webpack 4 structure and the reason why the charts module doesn't export anything?

AMoo-Miki · 2023-09-28T06:52:16Z

A few observations, after rebasing and resolving conflicts:

The updated autoprefixer causes duplicate CSS. I am not worried about this because i will be porting some changes from OSD that will eliminate the need for autoprefixer.
*.min.css files are no longer minified. postcss and cssnano have both been bumped but I didn't dig to see which portion is breaking the logic.
oui.js is 1.5MB larger; doesn't sound right and more investigation is needed.
oui.min.js, oui.min.js.LICENSE.txt, oui.min.js.map fail to generate.
*ui_charts_theme.js are generated as folders and not files.

I have pushed the rebased code to AMoo-Miki@ddf16ef.

AMoo-Miki · 2023-09-29T21:58:44Z

Fixed the minification:

Had to disable cssDeclarationSorter because it could have been unsafe.
The new cssnano merges back-to-back media queries and removes the media queries for the lower boundary; verified that it was safe.
The new cssnano also doesn't break and combine neighboring definitions.
rgba is converted to hsla; shouldn't be a big deal.
svgo produces slightly longer assets.

BSFishy · 2023-09-30T17:48:17Z

svgo produces slightly longer assets.

But the assets generated by svgo should all be in separate files right? All OUI icons are loaded in async, and won't be in the main bundle, right?

AMoo-Miki · 2023-10-01T00:11:44Z

But the assets generated by svgo should all be in separate files right? All OUI icons are loaded in async, and won't be in the main bundle, right?

OUI inlines them :/

postcss.config.js has postcss-inline-svg defined in it. However, the ones I saw elongated are those that it finds in imported deps.

AMoo-Miki · 2023-10-01T00:20:20Z

In an attempt to dig deeper into why the JS files were larger with webpack5, i split the eui_charts_theme.js files, before (260KB) and after (363KB). I see a lot more imports from corjs included with webpack5; 57 files totaling 100KB. These include stuff like:

../node_modules/core-js/internals/array-from.js
../node_modules/core-js/internals/iterator-define.js
../node_modules/core-js/internals/regexp-exec.js
../node_modules/core-js/modules/es.symbol.constructor.js

Also, one almost 5KB is wasted due to two indented copyright headers!

I suspect the same is happening with oui.js too.

My latest stuff can be found at https://github.com/AMoo-Miki/oui/tree/updated-webpack-5.

AMoo-Miki · 2023-10-01T01:16:05Z

With oui.js:

1.5MB was added.
197 imports of 461KB are removed.
- some removed imports are index.ts and types.ts files that only re-exported stuff:
```
./components/delay_hide/index.ts
./services/color_picker/index.ts
./services/popover/index.ts
```
62 imports of 214KB are added.
- imports from assert make up 114KB of that.
- 15 are from corejs worth 20KB.

1919 files have some kind of change in them, resulting in the 1.7MB delta:

Numerous files (i stopped counting at 50 of the first 100) have gained 5KB-10KB, all because of lines like:

/* harmony import */ var core_js_modules_es_array_slice_js__WEBPACK_IMPORTED_MODULE_21__ = __webpack_require__(/*! core-js/modules/es.array.slice.js */ "../node_modules/core-js/modules/es.array.slice.js");
/* harmony import */ var core_js_modules_es_array_slice_js__WEBPACK_IMPORTED_MODULE_21___default = /*#__PURE__*/__webpack_require__.n(core_js_modules_es_array_slice_js__WEBPACK_IMPORTED_MODULE_21__);

AMoo-Miki · 2023-10-02T19:01:50Z

by the way, i couldn't get the docs to work. yarn start shows a blank page.

BSFishy · 2023-10-03T16:01:15Z

by the way, i couldn't get the docs to work. yarn start shows a blank page.

From CI, it looks like there's still reference to raw-loader: https://github.com/opensearch-project/oui/actions/runs/6177159690/job/16767685139?pr=770#step:5:31

andreymyssak requested review from AMoo-Miki, BSFishy, KrooshalUX, ashwin-pc, bandinib-amzn, joshuarrrr, rednaksi91 and seanneumann as code owners May 20, 2023 15:20

github-actions bot added the valued-contributor label May 20, 2023

andreymyssak commented May 20, 2023

View reviewed changes

andreymyssak force-pushed the 584-update-webpack branch 2 times, most recently from 14ab77c to 029a8dd Compare May 21, 2023 07:47

andreymyssak commented May 21, 2023

View reviewed changes

BSFishy reviewed May 22, 2023

View reviewed changes

jmetev1 mentioned this pull request May 22, 2023

Update to Webpack 5 #765

Closed

6 tasks

andreymyssak force-pushed the 584-update-webpack branch from 47c290e to b0c56f3 Compare May 29, 2023 10:22

joshuarrrr assigned BSFishy Jun 14, 2023

andreymyssak and others added 7 commits July 13, 2023 09:50

Bump cssnano to 6.0.1 (opensearch-project#584)

5252cd4

Co-authored-by: Sergey Myssak <sergey.myssak@gmail.com> Signed-off-by: Andrey Myssak <andreymyssak@gmail.com>

Update devServer in webpack.config.js (opensearch-project#584)

21851bb

Co-authored-by: Sergey Myssak <sergey.myssak@gmail.com> Signed-off-by: Andrey Myssak <andreymyssak@gmail.com>

Set import/no-unresolved to off (opensearch-project#584)

9d4545b

Co-authored-by: Sergey Myssak <sergey.myssak@gmail.com> Signed-off-by: Andrey Myssak <andreymyssak@gmail.com>

Remove import/no-unresolved from .eslintrc.js (opensearch-project#584)

bd34706

Co-authored-by: Sergey Myssak <sergey.myssak@gmail.com> Signed-off-by: Andrey Myssak <andreymyssak@gmail.com>

Remove null from resolve() in icon.test.tsx (opensearch-project#584)

d7b1113

Co-authored-by: Sergey Myssak <sergey.myssak@gmail.com> Signed-off-by: Andrey Myssak <andreymyssak@gmail.com>

Update webpack commands in compile-oui.js (opensearch-project#584)

d8bfe20

Co-authored-by: Sergey Myssak <sergey.myssak@gmail.com> Signed-off-by: Andrey Myssak <andreymyssak@gmail.com>

Adjust resolutions (opensearch-project#584)

0f07baf

Co-authored-by: Sergey Myssak <sergey.myssak@gmail.com> Signed-off-by: Andrey Myssak <andreymyssak@gmail.com>

andreymyssak force-pushed the 584-update-webpack branch from b0c56f3 to 0f07baf Compare July 13, 2023 14:11

andreymyssak requested a review from SergeyMyssak as a code owner July 13, 2023 14:11

andreymyssak and others added 2 commits July 13, 2023 20:22

Replace 'master' with 'main' in the comment in changelog.js (opensear…

60f986e

…ch-project#584) Co-authored-by: Sergey Myssak <sergey.myssak@gmail.com> Signed-off-by: Andrey Myssak <andreymyssak@gmail.com>

Update CHANGELOG.md (opensearch-project#584)

ed5773e

Co-authored-by: Sergey Myssak <sergey.myssak@gmail.com> Signed-off-by: Andrey Myssak <andreymyssak@gmail.com>

andreymyssak requested a review from BSFishy July 14, 2023 05:13

joshuarrrr added 2 commits September 13, 2023 12:45

Merge branch 'main' into 584-update-webpack

22fb706

Signed-off-by: Josh Romero <rmerqg@amazon.com>

update yarn.lock

9306612

Signed-off-by: Josh Romero <rmerqg@amazon.com>

This was referenced Oct 5, 2023

[BUG] Fix console warnings about 'Query' export #1063

Closed

Audit unused packages #595

Open

BSFishy mentioned this pull request Oct 16, 2023

Update Babel #1098

Closed

7 tasks

		// eslint-disable-next-line @typescript-eslint/ban-ts-comment
		// @ts-ignore

	export { Query } from './query';
	export { AST } from './ast';

Conversation

andreymyssak commented May 20, 2023

Description

Issues Resolved

Check List

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreymyssak May 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BSFishy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreymyssak May 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

andreymyssak May 20, 2023 •

edited

Loading

andreymyssak May 29, 2023 •

edited

Loading

andreymyssak commented Jul 13, 2023 •

edited

Loading

BSFishy commented Jul 20, 2023 •

edited

Loading