perf: prioritize the main bundle over `<link rel=preload>`s #23556

iamakulov · 2023-05-19T16:10:32Z

Description

After #21933, the main bundle started to compete for bandwidth with the preloaded chunks. On pages without the preloads, the bundle would take ~2s to load (on 4G) (request no. 3):

whereas on pages with the chunks, it will take ~3s (request no. 13):

This PR fixes that by:

removing the defer attribute, which bumps the main bundle’s priority from Low to High (reference: https://addyosmani.com/blog/script-priorities/)
and setting fetchpriority="low" on the preload chunks

This changes the waterfall to look like this instead:

Type of change

Chore (housekeeping or task changes that don't impact user perception)

Testing

How Has This Been Tested?

Manual – tested different attributes in WebPageTest + implemented them locally without perf testing

Checklist:

Dev activity

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
PR is being merged under a feature flag

QA activity:

Speedbreak features have been covered
Test plan covers all impacted features and areas of interest
Test plan has been peer reviewed by project stakeholders and other QA members
Manually tested functionality on DP
We had an implementation alignment call with stakeholders post QA Round 2
Cypress test cases have been added and approved by SDET/manual QA
Added Test Plan Approved label after Cypress tests were reviewed
Added Test Plan Approved label after JUnit tests were reviewed

SatishGandham · 2023-05-22T06:29:57Z

/ok-to-test

github-actions · 2023-05-22T06:30:21Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/5042569965.
Workflow: Appsmith External Integration Test Workflow.
Commit: ``.
PR: 23556.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=23556&runId=5042569965_1

github-actions · 2023-05-29T16:35:28Z

This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected.

SatishGandham · 2023-05-30T06:13:39Z

/ok-to-test

github-actions · 2023-05-30T06:14:02Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/5118173965.
Workflow: Appsmith External Integration Test Workflow.
Commit: ``.
PR: 23556.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=23556&runId=5118173965_1

SatishGandham · 2023-05-30T06:31:19Z

app/client/craco.build.config.js

+ "htmlWebpackPlugin.userOptions must be defined",
+ );
+
+ htmlWebpackPlugin.userOptions.inject = "head";


@iamakulov , What happens if we put this main at the end of body? Does it have to compete for bandwidth with preloads?

I think this request was scheduled late-ish in my tests (due to being discovered later), and one or two <link rel="preload"> stole its bandwidth. I’ll re-run the tests and get back with waterfalls in a bit!

Yup, re-ran the tests. With <script> in the end of <head> (as in this PR), the main bundle takes 2.1s (request 5):

HTML code

<!doctype html> <html lang="en"> <head> <script>navigator.serviceWorker.register = () => { };</script> <meta charset="utf-8" /> <meta name="viewport" content="width=device-width,initial-scale=1,shrink-to-fit=no" /> <title>Appsmith</title> <style> #loader { position: fixed; left: 0; top: 0; height: 4px; background: #d7d7d7; transition: all ease-in .3s } </style> <link rel="preload" as="script" href="/static/js/2209.1a60b0b6.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/55178.ef82f381.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/81621.58d65afe.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/89532.5ad27563.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/85342.fd2d6599.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/3638.68e4f960.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/51863.ac47ac64.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/global-search.3bd25387.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/82492.1f73e40c.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/66124.a789a752.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/7207.9189ae78.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/29020.74dfd06a.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/72527.ce5b870f.chunk.js" fetchpriority="low" /> <link rel="preload" as="style" href="/static/css/editor.6f95ab40.chunk.css" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/editor.8486711d.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/2209.1a60b0b6.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/55178.ef82f381.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/51863.ac47ac64.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/global-search.3bd25387.chunk.js" fetchpriority="low" /> <script>const parseConfig = e => { if (0 === e.indexOf("__") || 0 === e.indexOf("$") || 0 === e.indexOf("%")) return ""; const o = e.trim(); return "false" !== o.toLowerCase() && "" !== o && ("true" === o.toLowerCase() || o) }, CLOUD_HOSTING = parseConfig("true"), ZIPY_KEY = parseConfig(""), AIRGAPPED = parseConfig("false"); if (CLOUD_HOSTING && ZIPY_KEY && !AIRGAPPED) { const e = document.createElement("script"); e.crossOrigin = "anonymous", e.defer = !0, e.src = "https://cdn.zipy.ai/sdk/v1.0/zipy.min.umd.js", e.onload = () => { window.zipy && window.zipy.init(ZIPY_KEY) }; const o = document.getElementsByTagName("head")[0]; o && o.appendChild(e) } gapiLoaded = () => { window.googleAPIsLoaded = !0 }, onError = () => { window.googleAPIsLoaded = !1 }</script> <script async defer="defer" id="googleapis" src="https://apis.google.com/js/api.js" onload="gapiLoaded()" onerror="onError()"></script> <script src="/static/js/main.c65af675.js"></script> <link href="/static/css/main.c3722dd4.css" rel="stylesheet"> </head> <body class="appsmith-light-theme"><noscript>You need to enable JavaScript to run this app.</noscript> <div id="loader" style="width:30vw"></div> <div id="header-root"></div> <div id="root"></div> <script type="text/javascript">const getIsLocalStorageSupported = () => { try { return window.localStorage.setItem("test", "testA"), window.localStorage.removeItem("test"), !0 } catch (e) { return !1 } }, isLocalStorageSupported = getIsLocalStorageSupported(), handleLocalStorageNotSupportedError = () => { console.error("Localstorage storage is not supported on your device.") }, localStorageUtil = { getItem: e => { if (isLocalStorageSupported) return window.localStorage.getItem(e); handleLocalStorageNotSupportedError() }, removeItem: e => { if (isLocalStorageSupported) return window.localStorage.removeItem(e); handleLocalStorageNotSupportedError() }, setItem: (e, r) => { if (isLocalStorageSupported) return window.localStorage.setItem(e, r); handleLocalStorageNotSupportedError() } }; window.addEventListener("DOMContentLoaded", (e => { document.getElementById("loader").style.width = "50vw" })); const registerPageServiceWorker = () => { "serviceWorker" in navigator && !window.Cypress && window.addEventListener("load", (function () { navigator.serviceWorker.register("/pageService.js").catch((e => { console.error("Service Worker Registration failed: " + e) })) })) }; "serviceWorker" in navigator && !window.Cypress && window.addEventListener("load", (function () { navigator.serviceWorker.register("/pageService.js").catch((e => { console.error("Service Worker Registration failed: " + e) })) }))</script> <script type="text/javascript">const LOG_LEVELS = ["debug", "error"], CONFIG_LOG_LEVEL_INDEX = LOG_LEVELS.indexOf(parseConfig("")), INTERCOM_APP_ID = parseConfig("%REACT_APP_INTERCOM_APP_ID%") || parseConfig("y10e7138"), DISABLE_INTERCOM = parseConfig("") || parseConfig("false"); INTERCOM_APP_ID.length && !DISABLE_INTERCOM && function () { var _ = window, e = _.Intercom; if ("function" == typeof e) e("reattach_activator"), e("update", _.intercomSettings); else { var E = document, a = function () { a.c(arguments) }; a.q = [], a.c = function (_) { a.q.push(_) }, _.Intercom = a; var I = function () { var _ = E.createElement("script"); _.type = "text/javascript", _.async = !0, _.src = "https://widget.intercom.io/widget/" + INTERCOM_APP_ID; var e = E.getElementsByTagName("script")[0]; e.parentNode.insertBefore(_, e) }; "complete" === document.readyState ? I() : _.attachEvent ? _.attachEvent("onload", I) : _.addEventListener("load", I, !1) } }(), window.SENTRY_CONFIG = parseConfig("https://abf15a075d1347969df44c746cca7eaa@o296332.ingest.sentry.io/1546547"), window.APPSMITH_FEATURE_CONFIGS = { sentry: { dsn: parseConfig("https://abf15a075d1347969df44c746cca7eaa@o296332.ingest.sentry.io/1546547"), release: parseConfig(""), environment: parseConfig("Production") }, smartLook: { id: parseConfig("c370af0df0edf38360adbefbdc47d2b42ea137c9") }, enableRapidAPI: parseConfig(""), segment: { apiKey: parseConfig("9OnZ6LnDztuZZo4zXfoutEEBB2wftHUH"), ceKey: parseConfig("AGuvNK0MfQgtxwBMCia7fRNxTjfoBRec") }, fusioncharts: { licenseKey: parseConfig("5bD2nuyE1C3C11D3E3E3C1G3F2I4B3C8xgaD3D2E3qG2A1A4dgrE2F4D1G-7xoD1D3D1lttA32B2B9C3B5D2C4B1D3H4A9kcC-13E3D4HH2sudB2D3D2A-9hH1CB5B8dfwB4G1H4I2A3C2C1D6B1E1C4G1C3B5o==") }, enableMixpanel: parseConfig("9OnZ6LnDztuZZo4zXfoutEEBB2wftHUH"), algolia: { apiId: parseConfig("AZ2Z9CJSJ0"), apiKey: parseConfig("dfde934d9bdc2e0b14830f1dd3cb240f"), indexName: parseConfig("omnibar_docusaurus_index") }, logLevel: CONFIG_LOG_LEVEL_INDEX > -1 ? LOG_LEVELS[CONFIG_LOG_LEVEL_INDEX] : LOG_LEVELS[1], cloudHosting: CLOUD_HOSTING, enableTNCPP: parseConfig("true"), appVersion: { id: parseConfig(""), releaseDate: parseConfig("") }, intercomAppID: INTERCOM_APP_ID, mailEnabled: parseConfig("true"), cloudServicesBaseUrl: parseConfig("https://cs.appsmith.com") || "https://cs.appsmith.com", googleRecaptchaSiteKey: parseConfig("6LfXDPIaAAAAAKUUoWxXi35KN7VIlXoD2rtI5gg8"), hideWatermark: parseConfig(""), disableIframeWidgetSandbox: parseConfig("true"), customerPortalUrl: parseConfig("") || "https://customer.appsmith.com", pricingUrl: parseConfig("") || "https://www.appsmith.com/pricing", airGapped: parseConfig("false") }</script> </body> </html>

With script in the end of <body>, the main bundle takes 3s (request 6):

HTML code

<!doctype html> <html lang="en"> <head> <script>navigator.serviceWorker.register = () => { };</script> <meta charset="utf-8" /> <meta name="viewport" content="width=device-width,initial-scale=1,shrink-to-fit=no" /> <title>Appsmith</title> <style> #loader { position: fixed; left: 0; top: 0; height: 4px; background: #d7d7d7; transition: all ease-in .3s } </style> <link rel="preload" as="script" href="/static/js/2209.1a60b0b6.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/55178.ef82f381.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/81621.58d65afe.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/89532.5ad27563.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/85342.fd2d6599.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/3638.68e4f960.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/51863.ac47ac64.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/global-search.3bd25387.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/82492.1f73e40c.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/66124.a789a752.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/7207.9189ae78.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/29020.74dfd06a.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/72527.ce5b870f.chunk.js" fetchpriority="low" /> <link rel="preload" as="style" href="/static/css/editor.6f95ab40.chunk.css" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/editor.8486711d.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/2209.1a60b0b6.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/55178.ef82f381.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/51863.ac47ac64.chunk.js" fetchpriority="low" /> <link rel="preload" as="script" href="/static/js/global-search.3bd25387.chunk.js" fetchpriority="low" /> <script>const parseConfig = e => { if (0 === e.indexOf("__") || 0 === e.indexOf("$") || 0 === e.indexOf("%")) return ""; const o = e.trim(); return "false" !== o.toLowerCase() && "" !== o && ("true" === o.toLowerCase() || o) }, CLOUD_HOSTING = parseConfig("true"), ZIPY_KEY = parseConfig(""), AIRGAPPED = parseConfig("false"); if (CLOUD_HOSTING && ZIPY_KEY && !AIRGAPPED) { const e = document.createElement("script"); e.crossOrigin = "anonymous", e.defer = !0, e.src = "https://cdn.zipy.ai/sdk/v1.0/zipy.min.umd.js", e.onload = () => { window.zipy && window.zipy.init(ZIPY_KEY) }; const o = document.getElementsByTagName("head")[0]; o && o.appendChild(e) } gapiLoaded = () => { window.googleAPIsLoaded = !0 }, onError = () => { window.googleAPIsLoaded = !1 }</script> <script async defer="defer" id="googleapis" src="https://apis.google.com/js/api.js" onload="gapiLoaded()" onerror="onError()"></script> <link href="/static/css/main.c3722dd4.css" rel="stylesheet"> </head> <body class="appsmith-light-theme"><noscript>You need to enable JavaScript to run this app.</noscript> <div id="loader" style="width:30vw"></div> <div id="header-root"></div> <div id="root"></div> <script type="text/javascript">const getIsLocalStorageSupported = () => { try { return window.localStorage.setItem("test", "testA"), window.localStorage.removeItem("test"), !0 } catch (e) { return !1 } }, isLocalStorageSupported = getIsLocalStorageSupported(), handleLocalStorageNotSupportedError = () => { console.error("Localstorage storage is not supported on your device.") }, localStorageUtil = { getItem: e => { if (isLocalStorageSupported) return window.localStorage.getItem(e); handleLocalStorageNotSupportedError() }, removeItem: e => { if (isLocalStorageSupported) return window.localStorage.removeItem(e); handleLocalStorageNotSupportedError() }, setItem: (e, r) => { if (isLocalStorageSupported) return window.localStorage.setItem(e, r); handleLocalStorageNotSupportedError() } }; window.addEventListener("DOMContentLoaded", (e => { document.getElementById("loader").style.width = "50vw" })); const registerPageServiceWorker = () => { "serviceWorker" in navigator && !window.Cypress && window.addEventListener("load", (function () { navigator.serviceWorker.register("/pageService.js").catch((e => { console.error("Service Worker Registration failed: " + e) })) })) }; "serviceWorker" in navigator && !window.Cypress && window.addEventListener("load", (function () { navigator.serviceWorker.register("/pageService.js").catch((e => { console.error("Service Worker Registration failed: " + e) })) }))</script> <script type="text/javascript">const LOG_LEVELS = ["debug", "error"], CONFIG_LOG_LEVEL_INDEX = LOG_LEVELS.indexOf(parseConfig("")), INTERCOM_APP_ID = parseConfig("%REACT_APP_INTERCOM_APP_ID%") || parseConfig("y10e7138"), DISABLE_INTERCOM = parseConfig("") || parseConfig("false"); INTERCOM_APP_ID.length && !DISABLE_INTERCOM && function () { var _ = window, e = _.Intercom; if ("function" == typeof e) e("reattach_activator"), e("update", _.intercomSettings); else { var E = document, a = function () { a.c(arguments) }; a.q = [], a.c = function (_) { a.q.push(_) }, _.Intercom = a; var I = function () { var _ = E.createElement("script"); _.type = "text/javascript", _.async = !0, _.src = "https://widget.intercom.io/widget/" + INTERCOM_APP_ID; var e = E.getElementsByTagName("script")[0]; e.parentNode.insertBefore(_, e) }; "complete" === document.readyState ? I() : _.attachEvent ? _.attachEvent("onload", I) : _.addEventListener("load", I, !1) } }(), window.SENTRY_CONFIG = parseConfig("https://abf15a075d1347969df44c746cca7eaa@o296332.ingest.sentry.io/1546547"), window.APPSMITH_FEATURE_CONFIGS = { sentry: { dsn: parseConfig("https://abf15a075d1347969df44c746cca7eaa@o296332.ingest.sentry.io/1546547"), release: parseConfig(""), environment: parseConfig("Production") }, smartLook: { id: parseConfig("c370af0df0edf38360adbefbdc47d2b42ea137c9") }, enableRapidAPI: parseConfig(""), segment: { apiKey: parseConfig("9OnZ6LnDztuZZo4zXfoutEEBB2wftHUH"), ceKey: parseConfig("AGuvNK0MfQgtxwBMCia7fRNxTjfoBRec") }, fusioncharts: { licenseKey: parseConfig("5bD2nuyE1C3C11D3E3E3C1G3F2I4B3C8xgaD3D2E3qG2A1A4dgrE2F4D1G-7xoD1D3D1lttA32B2B9C3B5D2C4B1D3H4A9kcC-13E3D4HH2sudB2D3D2A-9hH1CB5B8dfwB4G1H4I2A3C2C1D6B1E1C4G1C3B5o==") }, enableMixpanel: parseConfig("9OnZ6LnDztuZZo4zXfoutEEBB2wftHUH"), algolia: { apiId: parseConfig("AZ2Z9CJSJ0"), apiKey: parseConfig("dfde934d9bdc2e0b14830f1dd3cb240f"), indexName: parseConfig("omnibar_docusaurus_index") }, logLevel: CONFIG_LOG_LEVEL_INDEX > -1 ? LOG_LEVELS[CONFIG_LOG_LEVEL_INDEX] : LOG_LEVELS[1], cloudHosting: CLOUD_HOSTING, enableTNCPP: parseConfig("true"), appVersion: { id: parseConfig(""), releaseDate: parseConfig("") }, intercomAppID: INTERCOM_APP_ID, mailEnabled: parseConfig("true"), cloudServicesBaseUrl: parseConfig("https://cs.appsmith.com") || "https://cs.appsmith.com", googleRecaptchaSiteKey: parseConfig("6LfXDPIaAAAAAKUUoWxXi35KN7VIlXoD2rtI5gg8"), hideWatermark: parseConfig(""), disableIframeWidgetSandbox: parseConfig("true"), customerPortalUrl: parseConfig("") || "https://customer.appsmith.com", pricingUrl: parseConfig("") || "https://www.appsmith.com/pricing", airGapped: parseConfig("false") }</script> <script src="/static/js/main.c65af675.js"></script> </body> </html>

Also note that the script is in the end of <head> in the first test. We can move it to the beginning of <head>, but that’s

a more complex build setup (HTMLWebpackPlugin doesn’t support that as a flag),

doesn’t even change the bundle’s download time (req. 2):

SatishGandham · 2023-06-02T06:21:03Z

/ok-to-test

github-actions · 2023-06-02T06:28:56Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/5152566123.
Workflow: Appsmith External Integration Test Workflow.
Commit: ``.
PR: 23556.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=23556&runId=5152566123_1

…ndle

This ensures developers won’t write code that works in dev but not in prod

…eeze` to `initAndRenderApp`

iamakulov · 2023-06-03T21:40:19Z

Alright, so to merge this, we need to:

Merge fix(tooltip): don’t assume that document.body is already accessible when the bundle inits design-system#480
Publish the new version of design-system-old and update it in this branch
Re-run the tests (should pass once the new design-system-old is used)

SatishGandham · 2023-06-07T08:46:30Z

@iamakulov , We are no longer using design-system-old.
@tanvibhakta , @albinAppsmith can you work with Ivan to close this.

appsmithorg/design-system#480

tanvibhakta · 2023-06-08T06:37:53Z

/ok-to-test

tanvibhakta · 2023-06-08T06:38:03Z

/build-deploy-preview

shadabbuchh · 2023-06-09T04:58:16Z

Tested this PR.
No tooltips are appearing for any of the Editable text fields or for Filepicker v2. Please note that we do not see tooltips in release as well for these components.
https://www.loom.com/share/198e9e93a3f94c1c9d836865cfe3daa3

shadabbuchh · 2023-06-09T10:20:37Z

As discussed with @tanvibhakta this PR cannot be tested because:

Tooltips are not visible for Editable text & FilePicker v2
We are not able to see the title bar & hence the tooltips on page name cannot be tested.

iamakulov · 2023-06-09T10:23:24Z

Should I try adding tooltips to some components just for the test purposes?

Edit: Storybook should also, presumably, confirm whether tooltips still work. That wouldn’t be a proper end-to-end test, though.

iamakulov · 2023-06-09T10:43:11Z

We are not able to see the title bar

Wait hmmm no, this is what’s the actual issue with this PR (and also a thing that’s failing Cypress tests). This is weird, looking into this now.

This was happening because the bundle now loads in `<head>`, and `document.getElementById("header-root")` is now undefined when executed at the top level

iamakulov · 2023-06-09T11:28:03Z

Alright, fixed the header not being rendered (very silly omission; c22ac38) and checked that there are no more top-level document. calls in the repo.

The tooltip seems to work:

iamakulov · 2023-06-09T11:28:28Z

/ok-to-test

github-actions · 2023-06-09T11:28:53Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/5221379865.
Workflow: Appsmith External Integration Test Workflow.
Commit: ``.
PR: 23556.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=23556&runId=5221379865_1

tanvibhakta · 2023-06-09T13:05:51Z

The tooltip seems to work:

Perfect, that's all we need right now. I'll be merging this PR into the design-system-old and push a commit here when I have the new release version (Monday morning). Thanks!

tanvibhakta · 2023-06-09T13:08:07Z

However for your purposes, this PR should also be able to tell you whether this fix truly does shave off the time or not. So, you shouldn't be blocked any longer, and this PR shouldn't be a blocker as to why the tests are failing.

tanvibhakta · 2023-06-09T13:08:21Z

/build-deploy-preview

github-actions · 2023-06-09T13:08:45Z

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/5222241943.
Workflow: On demand build Docker image and deploy preview.
skip-tests: . env: .
PR: 23556.
recreate: .

github-actions · 2023-06-09T13:39:11Z

Deploy-Preview-URL: https://ce-23556.dp.appsmith.com

SatishGandham · 2023-06-12T06:04:11Z

/ok-to-test

github-actions · 2023-06-12T06:04:37Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/5240093603.
Workflow: Appsmith External Integration Test Workflow.
Commit: ``.
PR: 23556.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=23556&runId=5240093603_1

SatishGandham · 2023-06-12T06:57:03Z

/ok-to-test

github-actions · 2023-06-12T06:57:29Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/5240474843.
Workflow: Appsmith External Integration Test Workflow.
Commit: ``.
PR: 23556.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=23556&runId=5240474843_1

iamakulov · 2023-06-12T11:08:11Z

I’m extremely puzzled why this CI in this branch suddenly caught much more ESLint errors than in release, despite release also having these errors. In any case, fixing these errors.

iamakulov · 2023-06-12T11:14:13Z

Ah yes, e27e5ff bumped a bunch of versions in yarn.lock. ~~Not sure why exactly; might be the automatic yarn dedupe, might be something else?~~ Update: just tried to do the merge myself. There was a conflict in yarn.lock, so I assume the merge might’ve went haywire (maybe due to rm yarn.lock && yarn?).

…ndle

iamakulov · 2023-06-12T11:21:17Z

Ouch, that commit also seems to have broken a bunch of type checks (https://github.com/appsmithorg/appsmith/actions/runs/5242801442/jobs/9466660879). @SatishGandham I’m going to re-do the merge and erase your commit, apologies!

iamakulov · 2023-06-12T12:40:04Z

Oh well. So due to palantir/blueprint#3248, removing the defer attribute from the bundle <script> (while keeping it in <head>) means popovers won’t render.

The root issue is here:

https://github.com/palantir/blueprint/blob/3dcbb0113f7f58637dec57a85c445011d51bafb6/packages/core/src/components/portal/portal.tsx#L59

This code gets executed when the bundle runs for the first time – and sets container to null, as document.body is not defined yet at that moment.

Sounds like we’ll have to figure out a different approach to prioritize the bundle without making it render-blocking.

iamakulov · 2023-06-12T17:13:19Z

Superseded by #24374!

perf: prioritize the main bundle over <link rel=preload>s

0604ee4

iamakulov requested a review from SatishGandham May 19, 2023 16:10

SatishGandham previously approved these changes May 19, 2023

View reviewed changes

Make the bundle code agnostic to whether it executes in <body> or <head>

5d7d891

iamakulov dismissed SatishGandham’s stale review via 5d7d891 May 22, 2023 11:38

iamakulov mentioned this pull request May 22, 2023

fix(tooltip): don’t assume that document.body is already accessible when the bundle inits appsmithorg/design-system#480

Closed

12 tasks

github-actions bot added the Stale label May 29, 2023

Merge branch 'release' into perf/prioritize-bundle

2ceca1a

SatishGandham reviewed May 30, 2023

View reviewed changes

github-actions bot removed the Stale label May 30, 2023

Merge branch 'release' into perf/prioritize-bundle

3317cf1

appsmithorg deleted a comment from github-actions bot Jun 2, 2023

github-actions bot requested review from albinAppsmith and tanvibhakta June 3, 2023 19:31

iamakulov added 3 commits June 3, 2023 22:33

Merge remote-tracking branch 'origin/release' into perf/prioritize-bu…

f6612b2

…ndle

Remove the defer attribute during development as well

0cca5bf

This ensures developers won’t write code that works in dev but not in prod

Remove appInitializer() duplicated during merging + move `setAutoFr…

827855d

…eeze` to `initAndRenderApp`

update ds-old version

46e1168

appsmithorg/design-system#480

appsmithorg deleted a comment from github-actions bot Jun 8, 2023

iamakulov added 2 commits June 9, 2023 13:10

Merge branch 'release' into perf/prioritize-bundle

4263440

Fix the header not being rendered

c22ac38

This was happening because the bundle now loads in `<head>`, and `document.getElementById("header-root")` is now undefined when executed at the top level

appsmithorg deleted a comment from github-actions bot Jun 12, 2023

Merge remote-tracking branch 'origin/release' into perf/prioritize-bu…

64242b8

…ndle

iamakulov force-pushed the perf/prioritize-bundle branch from 859bb01 to 64242b8 Compare June 12, 2023 11:21

appsmithorg deleted a comment from github-actions bot Jun 12, 2023

iamakulov mentioned this pull request Jun 12, 2023

perf: prioritize the main bundle over preloads (v2) #24374

Merged

19 tasks

iamakulov closed this Jun 12, 2023

perf: prioritize the main bundle over <link rel=preload>s #23556

perf: prioritize the main bundle over <link rel=preload>s #23556

Conversation

iamakulov commented May 19, 2023 • edited Loading

Description

Type of change

Testing

How Has This Been Tested?

Checklist:

Dev activity

QA activity:

SatishGandham commented May 22, 2023

github-actions bot commented May 22, 2023

github-actions bot commented May 29, 2023

SatishGandham commented May 30, 2023

github-actions bot commented May 30, 2023

SatishGandham May 30, 2023

Choose a reason for hiding this comment

iamakulov Jun 2, 2023

Choose a reason for hiding this comment

iamakulov Jun 3, 2023 • edited Loading

Choose a reason for hiding this comment

iamakulov Jun 3, 2023

Choose a reason for hiding this comment

SatishGandham commented Jun 2, 2023

github-actions bot commented Jun 2, 2023

iamakulov commented Jun 3, 2023

SatishGandham commented Jun 7, 2023

tanvibhakta commented Jun 8, 2023

tanvibhakta commented Jun 8, 2023

shadabbuchh commented Jun 9, 2023

shadabbuchh commented Jun 9, 2023

iamakulov commented Jun 9, 2023 • edited Loading

iamakulov commented Jun 9, 2023

iamakulov commented Jun 9, 2023 • edited Loading

iamakulov commented Jun 9, 2023

github-actions bot commented Jun 9, 2023

tanvibhakta commented Jun 9, 2023

tanvibhakta commented Jun 9, 2023

tanvibhakta commented Jun 9, 2023

github-actions bot commented Jun 9, 2023

github-actions bot commented Jun 9, 2023

SatishGandham commented Jun 12, 2023

github-actions bot commented Jun 12, 2023

SatishGandham commented Jun 12, 2023

github-actions bot commented Jun 12, 2023

iamakulov commented Jun 12, 2023

iamakulov commented Jun 12, 2023 • edited Loading

iamakulov commented Jun 12, 2023

iamakulov commented Jun 12, 2023 • edited Loading

iamakulov commented Jun 12, 2023

perf: prioritize the main bundle over `<link rel=preload>`s #23556

perf: prioritize the main bundle over `<link rel=preload>`s #23556

iamakulov commented May 19, 2023 •

edited

Loading

iamakulov Jun 3, 2023 •

edited

Loading

iamakulov commented Jun 9, 2023 •

edited

Loading

iamakulov commented Jun 9, 2023 •

edited

Loading

iamakulov commented Jun 12, 2023 •

edited

Loading

iamakulov commented Jun 12, 2023 •

edited

Loading