[shared-ui-deps] should handle transitive deps?

[Dosant]

@kbn/shared-ui-deps is used so that most common dependencies across kibana UI are shared between all the plugins and loaded once (e.g. React, EUI, lodash, etc...)

However, when a plugin imports a dependency of one of @kbn/shared-ui-deps dependencies it doesn't get imported from a single @kbn/shared-ui-deps bundle, but it gets bundle into plugin's bundle.

For example:

• Some plugins imported directly from react-intl instead of @kbn/i18n/react. Fixing this shaved -386.0KB in total from async bundles
• Another example is react-beautiful-dnd which is a dependency of EUI: react-beautiful-dnd ~400K per each plugin that uses it

Since these transition deps are already bundled into @kbn/shared-ui-deps, when importing such in Kibana directly should they be imported from a shared bundle instead of bundling as a separate instance? If we fix this, the safe would likely be significant (looking at two existing known examples of such mentioned above)

[elasticmachine]

Pinging @elastic/kibana-operations (Team:Operations)

[mistic]

I think transitive deps should be already bundled. The only thing we need to do if we want to use them is expose them as externals.

[spalger]

Yeah, we need to explicitly choose which packages we want to expose via externals otherwise we'd be bound to the specific version of each transitive dependency used by the packages in @kbn/ui-shared-deps. Exposing transitive dependencies and automatically rewriting all imports for those packages to use ui-shared-deps is as easy as exposing them in two places:

• with a export const PackageName = require('transitive-dep') in packages/kbn-ui-shared-deps/src/entry.js
• with a mapping in the externals map at the bottom of packages/kbn-ui-shared-deps/src/index.js

[mistic]

@Dosant do you think we can consider this closed?

[Dosant]

@Dosant do you think we can consider this closed?

@mistic, I think these changes that @spalger laid out should be at least done for react-intl and react-beautiful-dnd before closing this? Ideally, in the scope of this, we also could identify other packages that could benefit from this? (maybe a script?)

Or are you suggesting this isn't worth pursuing?

[spalger]

I think that instead of sharing react-intl we should rely on ESLint to discourage it's use, but we'll see what the improvement of sharing react-beautiful-dnd is in #102834. I don't think it's clear how we'd write a script to determine what is or isn't worth sharing, but if you wanted to take a stab at it I'd be happy to lend a hand or walk you though how I might do it. Transient dependencies of packages already in ui-shared-deps are definitely a vein of dependencies that would probably be worth discovering and sharing since they're already loaded on every page anyway.

[spalger]

In #110558 we split the @kbn/ui-shared-deps package into two, one with the dependencies from NPM and one with the dependencies which are in the source. The @kbn/ui-shared-deps-src package works the same way the old package did by exposing an externals map which forces chosen modules to be overridden with the version from @kbn/ui-shared-deps-*. The @kbn/ui-shared-deps-npm package on the otherhand exposes a DLL which is referenced by the @kbn/ui-shared-deps-src package and all bundles created by the optimizer. This allows any NPM module which is already in that DLL to be linked from the bundles rather than copied into them, effectively handling transitive deps for the vast majority of packages. If there are additional packages we would like to add to that DLL we can do so, but for now I think we're in a good position to close this issue.