Consolidate webasset bundle to single chunks #38706

avatus · 2024-02-27T23:14:01Z

Refer to the "Why" section of this RFD for info on the issue solved with this PR.

This PR will configure vite to pipe all code chunks together. So techincally, this isn't removing code splitting, but in reality just not splitting the code. The benefits of this way are

we do not have to remove any of our dynamically imported code as it will just import it all at build time
in the future, we can take advantage of the code splitting function (right now its essentially an empty anonymous function) to have more fine grained control over the code splitting in the future.

This ignores fonts/images/wasm, and only affects javascript and css files. We will now have a single javascript and css file load on initial page load. This will increase latency slightly as there is a larger file to download but it will remove any other network requests for javascript files after (since it has the entire bundle) and usually the javascript file and css file will be cached by the browser anyway so this initial latency increase should only happen once per version (or however their browser may be set up).

Quick loom videos showing the error and fix
https://www.loom.com/share/bcae5d79b14a4e6e8df8e0603ace0f04
https://www.loom.com/share/2ce5386941c849dca6a142089d927aa3

avatus · 2024-02-28T04:14:18Z

Updated with some loom videos showing it work/not-work
https://www.loom.com/share/bcae5d79b14a4e6e8df8e0603ace0f04
https://www.loom.com/share/2ce5386941c849dca6a142089d927aa3

Please help me break this!

avatus · 2024-02-28T04:30:51Z

I did toy with the idea of keeping the hashing for index.js and index.css ONLY, just to make sure those are "in sync" since they are loaded at the same time. I think I went without just to make sure something is always loaded rather than a rare case that the instant one loads it load balances to the other version. Slightly broken css might not be as bad as "page didn't load at all, refresh"

ryanclark · 2024-02-28T15:06:33Z

I did toy with the idea of keeping the hashing for index.js and index.css ONLY, just to make sure those are "in sync" since they are loaded at the same time. I think I went without just to make sure something is always loaded rather than a rare case that the instant one loads it load balances to the other version. Slightly broken css might not be as bad as "page didn't load at all, refresh"

index.css only contains the fonts so it'll very rarely change

ryanclark · 2024-02-28T15:07:55Z

web/packages/build/vite/config.ts

+            // This removes code splitting.
+            inlineDynamicImports: true,


Can we remove React.lazy and all the default exports instead? Otherwise we're wrapping the loading of all route components in a promise for no reason

Removed react lazy on everything except access graph/assist. Was getting really weird "React Markdown" errors unless it was lazy loaded. like 40+ of them during testing. I'll keep inspecting but for now, every other lazy is removed

We could something like with manualChunks to programmatically choose which chunks are split and remove hashing that way for those specifically, or we can keep this line to handle the assist/access graph and call it a day. I dont know much about assist's internals I think to know what to chunk out

e update if we stick with these changes https://github.com/gravitational/teleport.e/pull/3566

avatus · 2024-02-28T18:54:45Z

Another loom of a "upgrade in process" with multiple sessions. There was an error that showed up but its because my second proxy couldn't reach the auth server (which is running with my first proxy) during an upgrade but thats not related to my webassets https://www.loom.com/share/1cd8dd618b054b63ac2462387ba32b07

avatus · 2024-02-29T14:47:39Z

Another way to help review this is to build the UI and just poke around to make sure everything works still. I made large changes to imports of features and while I tried to check most it'd be nice to have another set of eyes to check for regressions

ibeckermayer

The linked-to RFD is still open, and this change appears to be talked about as an aside at the end. Was it decided somewhere that this is the approach that we're going with? If yes, have we characterized the performance impact?

ibeckermayer · 2024-03-01T01:32:30Z

web/packages/build/vite/config.ts

@@ -69,6 +69,18 @@ export function createViteConfig(
        outDir: outputDirectory,
        assetsDir: 'app',
        emptyOutDir: true,
+        rollupOptions: {
+          output: {
+            // This removes code splitting.


Suggested change

// This removes code splitting.

// This prevents code splitting.

ibeckermayer · 2024-03-01T01:33:35Z

web/packages/build/vite/config.ts

+            // this will remove hashing from asset (non-js) files. We will keep the javascript file
+            // hashed in order to avoid a cache issue between versions. `no-cache` headers are set
+            // for our html file


We will keep the javascript file hashed

doesn't this contradict the previous comment of

removes hashing from our entry point file

?

sorry, an older version and I left the comments in. thanks

ibeckermayer · 2024-03-01T01:34:35Z

web/packages/teleport/src/Integrations/Enroll/AwsOidc/index.ts

@@ -16,5 +16,4 @@
 * along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */

-// export as default for use with React.lazy
 export { AwsOidc as default } from './AwsOidc';


Why are we keeping this one?

ibeckermayer · 2024-03-01T01:35:02Z

web/packages/teleport/src/LocksV2/NewLock/LockCheckout/index.ts

@@ -16,5 +16,4 @@
 * along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */

-// export as default for use with React.lazy
 export { LockCheckout as default } from './LockCheckout';


And this one?

ryanclark · 2024-03-04T09:22:22Z

web/packages/teleport/src/Teleport.tsx

+import LoginFailed from './Login/LoginFailed';
+import LoginSuccess from './Login/LoginSuccess';
+import Login from './Login';
+import Welcome from './Welcome';

-import type { History } from 'history';
+import Console from './Console';
+import Player from './Player';
+import DesktopSession from './DesktopSession';


These are all default exports

ryanclark · 2024-03-04T09:23:33Z

web/packages/teleport/src/TrustedClusters/index.ts

 import TrustedClusters from './TrustedClusters';
-export default TrustedClusters;
+export { TrustedClusters };


Can we use this PR to clean these up all the way through to the component?

That way we can have a consistent

export { TrustedClusters } from './TrustedClusters';

I know it's slightly increasing the scope of this PR, but we'd never get round to the cleanup otherwise

Feel free to ignore

ryanclark · 2024-03-04T09:23:47Z

web/packages/teleport/src/features.tsx

+import Account from './Account';
+import Support from './Support';


Still default

ryanclark

Approving as it LGTM, pending the last default export changes

This will prevent the react bundle from being code split. This means that our entire javascript bundle will be a single file as well as our css file. It also removes hashing from the included webassets. The reason for this change is to allow different versioned proxies to coexist behind a load balancer. If we remove code splitting, then the only time our bundle will load is on initial hit, and the web client will then have the entire bundle. This also removes React.lazy. React.lazy wrapped components in a promise to allow for dynamic imports during runtime. This is no longer needed. Lastly, to clean up the default exports used by react lazy, this removes (most) default exports for feature components. Assist and telemetry-boot are not included in the javascript bundle

public-teleport-github-review-bot · 2024-03-05T19:54:02Z

@avatus See the table below for backport results.

Branch	Result
branch/v15	Create PR

This will prevent the react bundle from being code split. This means that our entire javascript bundle will be a single file as well as our css file. It also removes hashing from the included webassets. The reason for this change is to allow different versioned proxies to coexist behind a load balancer. If we remove code splitting, then the only time our bundle will load is on initial hit, and the web client will then have the entire bundle. This also removes React.lazy. React.lazy wrapped components in a promise to allow for dynamic imports during runtime. This is no longer needed. Lastly, to clean up the default exports used by react lazy, this removes (most) default exports for feature components. Assist and telemetry-boot are not included in the javascript bundle

* Prevent code splitting (#38706) This will prevent the react bundle from being code split. This means that our entire javascript bundle will be a single file as well as our css file. It also removes hashing from the included webassets. The reason for this change is to allow different versioned proxies to coexist behind a load balancer. If we remove code splitting, then the only time our bundle will load is on initial hit, and the web client will then have the entire bundle. This also removes React.lazy. React.lazy wrapped components in a promise to allow for dynamic imports during runtime. This is no longer needed. Lastly, to clean up the default exports used by react lazy, this removes (most) default exports for feature components. Assist and telemetry-boot are not included in the javascript bundle * Fix the import on the account settings page (#39015) * Fix auth connector imports (#39019) * Init discover resources inside the discover component (#39166) * Use ETag headers for served javascript files (#39251) --------- Co-authored-by: Bartosz Leper <[email protected]>

avatus requested review from ryanclark, kimlisa and rudream February 27, 2024 23:14

avatus added the no-changelog Indicates that a PR does not require a changelog entry label Feb 27, 2024

github-actions bot requested a review from ibeckermayer February 27, 2024 23:14

avatus added backport-required backport/branch/v13 backport/branch/v14 backport/branch/v15 labels Feb 27, 2024

github-actions bot added size/sm ui labels Feb 27, 2024

avatus removed backport/branch/v13 backport/branch/v14 labels Feb 27, 2024

avatus force-pushed the avatus/remove_code_splitting branch from 1a4899b to c6317dc Compare February 28, 2024 04:13

ryanclark reviewed Feb 28, 2024

View reviewed changes

avatus force-pushed the avatus/remove_code_splitting branch from ff4b1ee to 5fc46ed Compare February 28, 2024 19:38

ibeckermayer reviewed Mar 1, 2024

View reviewed changes

avatus mentioned this pull request Mar 1, 2024

RFD 0159: webasset storage service #35757

Closed

avatus requested review from ryanclark and ibeckermayer March 1, 2024 14:43

ryanclark reviewed Mar 4, 2024

View reviewed changes

ryanclark approved these changes Mar 4, 2024

View reviewed changes

rudream approved these changes Mar 4, 2024

View reviewed changes

public-teleport-github-review-bot bot removed the request for review from kimlisa March 4, 2024 19:23

avatus added this pull request to the merge queue Mar 4, 2024

avatus removed this pull request from the merge queue due to a manual request Mar 4, 2024

avatus force-pushed the avatus/remove_code_splitting branch 5 times, most recently from 10bf931 to 0159477 Compare March 5, 2024 17:05

avatus force-pushed the avatus/remove_code_splitting branch from 0159477 to 82d1a4e Compare March 5, 2024 17:18

kimlisa approved these changes Mar 5, 2024

View reviewed changes

avatus added this pull request to the merge queue Mar 5, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 5, 2024

avatus added this pull request to the merge queue Mar 5, 2024

Merged via the queue into master with commit e922db0 Mar 5, 2024
34 checks passed

avatus deleted the avatus/remove_code_splitting branch March 5, 2024 19:52

avatus mentioned this pull request Mar 5, 2024

[v15] Consolidate webasset bundle to single chunks #38989

Merged

bl-nero mentioned this pull request Mar 6, 2024

Fix the import on the account settings page #39015

Merged

flyinghermit mentioned this pull request Mar 10, 2024

SAML app discover tile not showing up in enterprise build #39163

Closed

avatus mentioned this pull request Mar 11, 2024

[v15] Consolidate webasset bundle to single chunks #39198

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consolidate webasset bundle to single chunks #38706

Consolidate webasset bundle to single chunks #38706

avatus commented Feb 27, 2024 •

edited

Loading

avatus commented Feb 28, 2024

avatus commented Feb 28, 2024

ryanclark commented Feb 28, 2024

ryanclark Feb 28, 2024

avatus Feb 28, 2024

avatus Feb 28, 2024

avatus Feb 28, 2024

avatus commented Feb 28, 2024

avatus commented Feb 29, 2024

ibeckermayer left a comment

ibeckermayer Mar 1, 2024

ibeckermayer Mar 1, 2024

avatus Mar 1, 2024

ibeckermayer Mar 1, 2024

ibeckermayer Mar 1, 2024

ryanclark Mar 4, 2024

ryanclark Mar 4, 2024

ryanclark Mar 4, 2024

ryanclark left a comment

public-teleport-github-review-bot bot commented Mar 5, 2024

	// This removes code splitting.
	// This prevents code splitting.

		import Account from './Account';
		import Support from './Support';

Consolidate webasset bundle to single chunks #38706

Consolidate webasset bundle to single chunks #38706

Conversation

avatus commented Feb 27, 2024 • edited Loading

avatus commented Feb 28, 2024

avatus commented Feb 28, 2024

ryanclark commented Feb 28, 2024

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

avatus commented Feb 28, 2024

avatus commented Feb 29, 2024

ibeckermayer 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanclark left a comment

Choose a reason for hiding this comment

public-teleport-github-review-bot bot commented Mar 5, 2024

avatus commented Feb 27, 2024 •

edited

Loading