Use `target_web` to ensure browser compatibility #130874

afharo · 2022-04-25T10:35:03Z

Summary

Because of how our bundle process works, any @kbn/* package used in the UI should build a web-optimized version target_web. Using the target_node build risks introducing compatibility issues as only target_web applies the browserlist compatibility-matrix necessary polyfills.

This PR adds the script node scripts/find_target_node_imports_in_bundles to list all packages imported in the UI with the default target_node build.

It also attempts to fix all the packages found in the script or removes the UI dependency on that package.

Related to #88678.

Highlights

`@kbn/server-route-repository`

@kbn/server-route-repository could leak @kbn/config-schema if imported from the UI. Plugins used a workaround to only import the method they used and enrich the typing with a 2nd import + type casting:

kibana/x-pack/plugins/apm/public/services/rest/create_call_apm_api.ts

Lines 9 to 20 in 50bd2cc

    
           import type { 
        
             ClientRequestParamsOf, 
        
             formatRequest as formatRequestType, 
        
             ReturnOf, 
        
             RouteRepositoryClient, 
        
             ServerRouteRepository, 
        
           } from '@kbn/server-route-repository'; 
        
           // @ts-expect-error cannot find module or correspondent type declarations 
        
           // The code and types are at separated folders on @kbn/server-route-repository 
        
           // so in order to do targeted imports they must me imported separately, and 
        
           // an error is expected here 
        
           import { formatRequest } from '@kbn/server-route-repository/target_node/format_request';

This PR creates a new browser-specific entry point to the package that only exports the utilities that are meant to be used in the UI. Thanks to these changes, the package can be imported from the UI without the risk of leaking server-side code.

Plugin `cloud_security_posture`

It was accidentally importing @kbn/config-schema to the UI because it imported a constant from the same file a validation schema was defined.

`@kbn/optimizer` and `kbn-ui-shared-deps-npm`

After the changes, running the script still brings 2 uses of target_node that I was not able to remove:

➜ node scripts/find_target_node_imports_in_bundles.js
 info analyzing 130 stats files
 succ found 2 @kbn/*/target_node imports in entry bundles and chunks
'@kbn/optimizer',
'kbn-ui-shared-deps-npm',

@kbn/optimizer is found in all the bundles because of the entry_point_creator.js injected via its webpack config.
kbn-ui-shared-deps-npm: I'm not entirely sure what is bringing this one in. Especially, that it's not @kbn/ui-shared-deps-npm.

@elastic/kibana-operations can you help me out with these? 🙏

Open question

Should we add a CI check to validate that we don't use target_node builds in our bundles?

For maintainers

This was checked for breaking API changes and was labeled appropriately

afharo · 2022-04-25T15:11:30Z

The page load increase triggered an investigation about how target_web builds are generated. #130904 should improve this.

afharo · 2022-04-28T15:53:10Z

packages/kbn-optimizer/src/index.ts

@@ -14,4 +14,4 @@ export * from './node';
 export * from './limits';
 export * from './cli';
 export * from './report_optimizer_timings';
-export * from './babel_runtime_helpers';
+export * from './audit_bundle_dependencies';


rename the dir because it no-longer contained only the logic for babel_runtime_helpers 😇

afharo · 2022-04-28T15:54:19Z

packages/kbn-server-route-repository/BUILD.bazel

@@ -52,6 +52,13 @@ jsts_transpiler(
  build_pkg_name = package_name(),
 )

+jsts_transpiler(
+  name = "target_web",
+  srcs = SRCS,


I wonder if a potential improvement could be to only include in this version the formatRequest helper to avoid the split imports in the plugins.

I went ahead and added only the needed files because jest complained that it couldn't reach the dependency ./parse_endpoint because it was from outside the module (from the import import { formatRequest } from '@kbn/server-route-repository/target_web/format_request';)

afharo · 2022-04-28T15:55:58Z

x-pack/plugins/cloud_security_posture/common/schemas/csp_rule.ts

@@ -6,8 +6,6 @@
 */
 import { schema as rt, TypeOf } from '@kbn/config-schema';

-export const cspRuleAssetSavedObjectType = 'csp_rule';


Moved to constants because importing this constant from here was also importing the bundle @kbn/config-schema.

elasticmachine · 2022-04-29T07:55:33Z

Pinging @elastic/apm-ui (Team:apm)

…repository

dgieselaar

Thanks, page load bundle improvements look pretty good!

nreese

maps changes LGTM
code review

mattkime · 2022-05-02T13:18:12Z

packages/kbn-field-types/BUILD.bazel

+  build_pkg_name = package_name(),
+)
+
+jsts_transpiler(


looks like this was repeated.

@mattkime we are intentionally transpiling twice with different names and Babel presets:

target_node - optimized for Node.js environment

target_web - optimized for running in browsers (mind the additional parameter web = True). Among other things, it applies the necessary transpilation and polyfills to support ourbrowserlist compatibility matrix.

If your package only runs on the UI and you think target_node is not necessary, we can discuss with @elastic/kibana-operations if it's ok to remove it or if target_node must always exist.

ah, not duplicate!

afharo · 2022-04-29T08:00:14Z

packages/kbn-server-route-repository/src/web_index.ts

@@ -0,0 +1,21 @@
+/*


This file is a different entry point exporting the web-only APIs.

afharo · 2022-05-02T13:53:39Z

packages/kbn-field-types/BUILD.bazel

+  build_pkg_name = package_name(),
+)
+
+jsts_transpiler(


@mattkime we are intentionally transpiling twice with different names and Babel presets:

target_node - optimized for Node.js environment

target_web - optimized for running in browsers (mind the additional parameter web = True). Among other things, it applies the necessary transpilation and polyfills to support ourbrowserlist compatibility matrix.

If your package only runs on the UI and you think target_node is not necessary, we can discuss with @elastic/kibana-operations if it's ok to remove it or if target_node must always exist.

shahzad31

Uptime changes LGTM !!

…e-bundle-improvements-target_web

kibana-ci · 2022-05-04T18:09:14Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`apm`	1179	1180	+1
`cloudSecurityPosture`	245	183	-62
`lists`	299	300	+1
`observability`	393	394	+1
`securitySolution`	2796	2797	+1
`ux`	132	133	+1
total			-57

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`apm`	2.8MB	2.8MB	-7.4KB
`canvas`	1.0MB	1020.1KB	-4.5KB
`cases`	290.2KB	290.1KB	-40.0B
`cloud`	18.6KB	18.6KB	+26.0B
`cloudSecurityPosture`	393.8KB	180.2KB	-213.6KB
`console`	399.5KB	396.1KB	-3.4KB
`core`	136.1KB	136.1KB	-18.0B
`dataViewFieldEditor`	148.0KB	147.9KB	-23.0B
`dataViewManagement`	124.0KB	121.2KB	-2.8KB
`dataVisualizer`	539.6KB	536.0KB	-3.6KB
`expressions`	25.6KB	25.6KB	-9.0B
`infra`	1002.7KB	1002.6KB	-68.0B
`lens`	1.1MB	1.1MB	-9.0KB
`lists`	147.7KB	147.0KB	-764.0B
`maps`	2.5MB	2.5MB	-120.0B
`ml`	3.3MB	3.3MB	-6.4KB
`observability`	443.0KB	434.2KB	-8.8KB
`presentationUtil`	136.8KB	127.3KB	-9.5KB
`searchprofiler`	164.2KB	160.6KB	-3.5KB
`securitySolution`	4.8MB	4.8MB	-5.7KB
`stackAlerts`	202.8KB	199.2KB	-3.6KB
`synthetics`	756.3KB	756.2KB	-95.0B
`transform`	374.8KB	371.2KB	-3.6KB
`triggersActionsUi`	727.7KB	723.4KB	-4.2KB
`unifiedSearch`	157.2KB	151.4KB	-5.8KB
`ux`	162.2KB	154.0KB	-8.2KB
`visTypeVega`	1.7MB	1.7MB	-211.0B
`watcher`	272.5KB	269.0KB	-3.5KB
total			-308.5KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`apm`	31.1KB	26.9KB	-4.2KB
`cases`	97.7KB	93.6KB	-4.1KB
`cloud`	10.1KB	10.1KB	-4.0B
`cloudSecurityPosture`	4.6KB	4.6KB	-1.0B
`core`	288.9KB	293.1KB	+4.2KB
`data`	416.5KB	413.7KB	-2.8KB
`dataViewFieldEditor`	24.4KB	21.7KB	-2.7KB
`dataViews`	42.2KB	39.7KB	-2.5KB
`expressions`	98.8KB	96.6KB	-2.2KB
`fieldFormats`	47.8KB	44.4KB	-3.3KB
`infra`	88.1KB	83.9KB	-4.1KB
`observability`	92.0KB	91.9KB	-171.0B
`securitySolution`	249.3KB	246.5KB	-2.8KB
`synthetics`	25.7KB	21.4KB	-4.2KB
`timelines`	275.9KB	271.8KB	-4.1KB
total			-32.9KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/optimizer`	46	47	+1

History

💔 Build #42515 failed 8899a1c
💚 Build #41388 succeeded 5f07ca9
💛 Build #41344 was flaky 1ef7722
💔 Build #41291 failed f9571447aba61c93998b01ce04a0ae9f9d2c9545
💔 Build #41256 failed 9753ccbc234ba7cb95eb1d85737469290658b51e

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

gchaps

LGTM from docs

afharo changed the base branch from afharo/spacetime-bundle-improvements to main April 25, 2022 11:29

afharo force-pushed the afharo/spacetime-bundle-improvements-target_web branch 2 times, most recently from 88770df to 9484183 Compare April 28, 2022 15:50

afharo commented Apr 28, 2022

View reviewed changes

afharo force-pushed the afharo/spacetime-bundle-improvements-target_web branch 3 times, most recently from 9753ccb to f957144 Compare April 28, 2022 19:29

Use target_web to improve bundle size and performance

1ef7722

afharo force-pushed the afharo/spacetime-bundle-improvements-target_web branch from f957144 to 1ef7722 Compare April 29, 2022 00:34

afharo marked this pull request as ready for review April 29, 2022 07:19

afharo requested review from a team as code owners April 29, 2022 07:19

Add JSDocs for the new script

45d6698

botelastic bot added the Team:APM All issues that need APM UI Team support label Apr 29, 2022

pgayvallet approved these changes Apr 29, 2022

View reviewed changes

Add entry point split explanation to the README in @kbn/server-route-…

5f07ca9

…repository

afharo changed the title ~~Use target_web to improve bundle size and performance~~ Use target_web to improve browser compatibility Apr 29, 2022

afharo changed the title ~~Use target_web to improve browser compatibility~~ Use target_web to ensure browser compatibility Apr 29, 2022

afharo mentioned this pull request Apr 29, 2022

[Meta] Reducing Kibana bundle sizes #88678

Open

37 tasks

dgieselaar approved these changes Apr 29, 2022

View reviewed changes

nreese approved these changes Apr 29, 2022

View reviewed changes

orouz approved these changes May 1, 2022

View reviewed changes

mattkime requested changes May 2, 2022

View reviewed changes

afharo commented May 4, 2022

View reviewed changes

shahzad31 approved these changes May 4, 2022

View reviewed changes

mattkime approved these changes May 4, 2022

View reviewed changes

afharo added 2 commits May 4, 2022 17:20

Merge branch 'main' of github.com:elastic/kibana into afharo/spacetim…

8899a1c

…e-bundle-improvements-target_web

Fix imports after merge from main

97fc3f8

spalger approved these changes May 4, 2022

View reviewed changes

yuliacech approved these changes May 5, 2022

View reviewed changes

gchaps approved these changes May 5, 2022

View reviewed changes

yctercero approved these changes May 5, 2022

View reviewed changes

afharo merged commit f7a1739 into elastic:main May 5, 2022

afharo deleted the afharo/spacetime-bundle-improvements-target_web branch May 5, 2022 19:20

kertal pushed a commit to kertal/kibana that referenced this pull request May 24, 2022

Use target_web to ensure browser compatibility (elastic#130874)

d7afa17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `target_web` to ensure browser compatibility #130874

Use `target_web` to ensure browser compatibility #130874

afharo commented Apr 25, 2022 •

edited

Loading

afharo commented Apr 25, 2022

afharo Apr 28, 2022

afharo Apr 28, 2022

afharo Apr 28, 2022

afharo Apr 28, 2022 •

edited

Loading

elasticmachine commented Apr 29, 2022

dgieselaar left a comment

nreese left a comment

mattkime May 2, 2022

afharo May 2, 2022

mattkime May 4, 2022

afharo Apr 29, 2022

afharo May 2, 2022

shahzad31 left a comment

kibana-ci commented May 4, 2022

API count

gchaps left a comment

	import type {
	ClientRequestParamsOf,
	formatRequest as formatRequestType,
	ReturnOf,
	RouteRepositoryClient,
	ServerRouteRepository,
	} from '@kbn/server-route-repository';
	// @ts-expect-error cannot find module or correspondent type declarations
	// The code and types are at separated folders on @kbn/server-route-repository
	// so in order to do targeted imports they must me imported separately, and
	// an error is expected here
	import { formatRequest } from '@kbn/server-route-repository/target_node/format_request';

Use target_web to ensure browser compatibility #130874

Use target_web to ensure browser compatibility #130874

Conversation

afharo commented Apr 25, 2022 • edited Loading

Summary

Highlights

@kbn/server-route-repository

Plugin cloud_security_posture

@kbn/optimizer and kbn-ui-shared-deps-npm

Open question

For maintainers

afharo commented Apr 25, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afharo Apr 28, 2022 • edited Loading

Choose a reason for hiding this comment

elasticmachine commented Apr 29, 2022

dgieselaar left a comment

Choose a reason for hiding this comment

nreese left a 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

shahzad31 left a comment

Choose a reason for hiding this comment

kibana-ci commented May 4, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

API count

History

gchaps left a comment

Choose a reason for hiding this comment

Use `target_web` to ensure browser compatibility #130874

Use `target_web` to ensure browser compatibility #130874

afharo commented Apr 25, 2022 •

edited

Loading

`@kbn/server-route-repository`

Plugin `cloud_security_posture`

`@kbn/optimizer` and `kbn-ui-shared-deps-npm`

afharo Apr 28, 2022 •

edited

Loading