From 345f51bedb145fed3d7ab88bc6808ad6e49d4347 Mon Sep 17 00:00:00 2001 From: Alf Eaton Date: Mon, 27 Nov 2023 11:26:06 +0000 Subject: [PATCH] [ide-react] Improve initial loading behaviour (#15916) * Defer script loading * Only mount IdePage once everything has connected GitOrigin-RevId: 32f16214f26ac6a6d71a9dd332b3c35b8b82deae --- services/web/app/views/layout-base.pug | 24 +++-- services/web/app/views/project/ide-react.pug | 5 +- .../ide-react/components/ide-root.tsx | 16 +-- .../features/ide-react/components/loading.tsx | 100 +++++++++--------- services/web/frontend/js/pages/ide.jsx | 21 ---- services/web/frontend/js/pages/ide.tsx | 6 ++ .../js/shared/hooks/use-wait-for-i18n.ts | 12 ++- 7 files changed, 85 insertions(+), 99 deletions(-) delete mode 100644 services/web/frontend/js/pages/ide.jsx create mode 100644 services/web/frontend/js/pages/ide.tsx diff --git a/services/web/app/views/layout-base.pug b/services/web/app/views/layout-base.pug index bd31e71e27..57ade4700a 100644 --- a/services/web/app/views/layout-base.pug +++ b/services/web/app/views/layout-base.pug @@ -72,7 +72,7 @@ html( body(ng-csp=(cspEnabled ? "no-unsafe-eval" : false) class=(showThinFooter ? 'thin-footer' : undefined)) if(settings.recaptcha && settings.recaptcha.siteKeyV3) - script(type="text/javascript", nonce=scriptNonce, src="https://www.recaptcha.net/recaptcha/api.js?render="+settings.recaptcha.siteKeyV3) + script(type="text/javascript", nonce=scriptNonce, src="https://www.recaptcha.net/recaptcha/api.js?render="+settings.recaptcha.siteKeyV3, defer=deferScripts) if (typeof(suppressSkipToContent) == "undefined") a(class="skip-to-content" href="#main-content") #{translate('skip_to_content')} @@ -84,17 +84,19 @@ html( block foot-scripts each file in entrypointScripts(entrypoint) - script(type="text/javascript", nonce=scriptNonce, src=file) + script(type="text/javascript", nonce=scriptNonce, src=file, defer=deferScripts) if (settings.splitTest.devToolbar.enabled) each file in entrypointScripts("devToolbar") - script(type="text/javascript", nonce=scriptNonce, src=file) + script(type="text/javascript", nonce=scriptNonce, src=file, defer=deferScripts) script(type="text/javascript", nonce=scriptNonce). - //- Look for bundle - var cdnBlocked = typeof Frontend === 'undefined' - //- Prevent loops - var noCdnAlreadyInUrl = window.location.href.indexOf("nocdn=true") != -1 - if (cdnBlocked && !noCdnAlreadyInUrl && navigator.userAgent.indexOf("Googlebot") == -1) { - //- Set query param, server will not set CDN url - window.location.search += "&nocdn=true"; - } + window.addEventListener('DOMContentLoaded', function() { + //- Look for bundle + var cdnBlocked = typeof Frontend === 'undefined' + //- Prevent loops + var noCdnAlreadyInUrl = window.location.href.indexOf("nocdn=true") != -1 + if (cdnBlocked && !noCdnAlreadyInUrl && navigator.userAgent.indexOf("Googlebot") == -1) { + //- Set query param, server will not set CDN url + window.location.search += "&nocdn=true"; + } + }) diff --git a/services/web/app/views/project/ide-react.pug b/services/web/app/views/project/ide-react.pug index 3d7b5213e6..83cd038d5b 100644 --- a/services/web/app/views/project/ide-react.pug +++ b/services/web/app/views/project/ide-react.pug @@ -4,6 +4,7 @@ block vars - var suppressNavbar = true - var suppressFooter = true - var suppressSkipToContent = true + - var deferScripts = true - metadata.robotsNoindexNofollow = true block entrypointVar @@ -24,5 +25,5 @@ block append meta block prepend foot-scripts each file in (useOpenTelemetry ? entrypointScripts("tracing") : []) - script(type="text/javascript", nonce=scriptNonce, src=file) - script(type="text/javascript", nonce=scriptNonce, src=(wsUrl || '/socket.io') + '/socket.io.js') + script(type="text/javascript", nonce=scriptNonce, src=file, defer=deferScripts) + script(type="text/javascript", nonce=scriptNonce, src=(wsUrl || '/socket.io') + '/socket.io.js', defer=deferScripts) diff --git a/services/web/frontend/js/features/ide-react/components/ide-root.tsx b/services/web/frontend/js/features/ide-react/components/ide-root.tsx index e9cd964b4b..7e18e43cb6 100644 --- a/services/web/frontend/js/features/ide-react/components/ide-root.tsx +++ b/services/web/frontend/js/features/ide-react/components/ide-root.tsx @@ -1,24 +1,16 @@ +import { FC, useState } from 'react' import { GenericErrorBoundaryFallback } from '@/shared/components/generic-error-boundary-fallback' import withErrorBoundary from '@/infrastructure/error-boundary' import IdePage from '@/features/ide-react/components/layout/ide-page' import { ReactContextRoot } from '@/features/ide-react/context/react-context-root' import { Loading } from '@/features/ide-react/components/loading' -import getMeta from '@/utils/meta' -function IdeRoot() { - // Check that we haven't inadvertently loaded Angular - // TODO: Remove this before rolling out this component to any users - if (typeof window.angular !== 'undefined') { - throw new Error('Angular detected. This page must not load Angular.') - } - - const loadingText = getMeta('ol-loadingText') +const IdeRoot: FC = () => { + const [loaded, setLoaded] = useState(false) return ( - - - + {loaded ? : } ) } diff --git a/services/web/frontend/js/features/ide-react/components/loading.tsx b/services/web/frontend/js/features/ide-react/components/loading.tsx index a65f33263a..2d886e2cac 100644 --- a/services/web/frontend/js/features/ide-react/components/loading.tsx +++ b/services/web/frontend/js/features/ide-react/components/loading.tsx @@ -1,69 +1,69 @@ import { FC, useEffect, useState } from 'react' -import LoadingBranded from '../../../shared/components/loading-branded' -import i18n from '../../../i18n' -import { useConnectionContext } from '../context/connection-context' +import LoadingBranded from '@/shared/components/loading-branded' +import useWaitForI18n from '@/shared/hooks/use-wait-for-i18n' import getMeta from '@/utils/meta' +import { useConnectionContext } from '../context/connection-context' +import { useIdeReactContext } from '@/features/ide-react/context/ide-react-context' -type LoadStatus = 'initial' | 'rendered' | 'connected' | 'loaded' +type Part = 'initial' | 'render' | 'connection' | 'translations' | 'project' -const loadProgressPercentage: Record = { - initial: 20, - rendered: 40, - connected: 70, - loaded: 100, -} +const initialParts = new Set(['initial']) + +const totalParts = new Set([ + 'initial', + 'render', + 'connection', + 'translations', + 'project', +]) + +export const Loading: FC<{ + setLoaded: (value: boolean) => void +}> = ({ setLoaded }) => { + const [loadedParts, setLoadedParts] = useState(initialParts) + + const progress = (loadedParts.size / totalParts.size) * 100 + + useEffect(() => { + setLoaded(progress === 100) + }, [progress, setLoaded]) -// Pass in loading text from the server because i18n will not be ready initially -export const Loading: FC<{ loadingText: string }> = ({ - loadingText, - children, -}) => { - const [loadStatus, setLoadStatus] = useState('initial') const { connectionState, isConnected } = useConnectionContext() - const loadProgress = loadProgressPercentage[loadStatus] - const editorLoaded = loadStatus === 'loaded' + const i18n = useWaitForI18n() + const { projectJoined } = useIdeReactContext() - const [i18nLoaded, setI18nLoaded] = useState(false) - const [translationLoadError, setTranslationLoadError] = useState(false) - - // Advance to 40% once this component is rendered useEffect(() => { - // Force a reflow now so that the animation from 20% to 40% occurs - // eslint-disable-next-line no-void - void document.body.offsetHeight - setLoadStatus('rendered') + setLoadedParts(value => new Set(value).add('render')) }, []) useEffect(() => { - i18n - .then(() => setI18nLoaded(true)) - .catch(() => { - setTranslationLoadError(true) - }) - }, []) - - useEffect(() => { - if (editorLoaded) { - return - } if (isConnected) { - setLoadStatus(i18nLoaded ? 'loaded' : 'connected') + setLoadedParts(value => new Set(value).add('connection')) } - }, [i18nLoaded, editorLoaded, setLoadStatus, isConnected]) + }, [isConnected]) - const translationLoadErrorMessage = translationLoadError - ? getMeta('ol-translationLoadErrorMessage') - : '' + useEffect(() => { + if (i18n.isReady) { + setLoadedParts(value => new Set(value).add('translations')) + } + }, [i18n.isReady]) - return editorLoaded ? ( - <>{children} - ) : ( + useEffect(() => { + if (projectJoined) { + setLoadedParts(value => new Set(value).add('project')) + } + }, [projectJoined]) + + const error = + connectionState.error || + (i18n.error ? getMeta('ol-translationLoadErrorMessage') : '') + + // Use loading text from the server, because i18n will not be ready initially + const label = getMeta('ol-loadingText') + + return (
- +
) } diff --git a/services/web/frontend/js/pages/ide.jsx b/services/web/frontend/js/pages/ide.jsx deleted file mode 100644 index ad000e06b3..0000000000 --- a/services/web/frontend/js/pages/ide.jsx +++ /dev/null @@ -1,21 +0,0 @@ -// Configure dynamically loaded assets (via webpack) to be downloaded from CDN -import '../utils/webpack-public-path' - -// Set up error reporting, including Sentry -import '../infrastructure/error-reporter' - -import ReactDOM from 'react-dom' -import IdeRoot from '../features/ide-react/components/ide-root' - -const element = document.getElementById('ide-root') -if (element) { - // Remove loading screen provided by the server and replace it with the same - // screen rendered in React. Could use replaceChildren() instead but browser - // support is relatively recent (arrived in Safari in 2020) - element.textContent = '' - - // This will not be valid in React 18, which has a new API. See - // https://github.com/reactwg/react-18/discussions/5 - // https://react.dev/blog/2022/03/08/react-18-upgrade-guide#deprecations - ReactDOM.render(, element) -} diff --git a/services/web/frontend/js/pages/ide.tsx b/services/web/frontend/js/pages/ide.tsx new file mode 100644 index 0000000000..c456139973 --- /dev/null +++ b/services/web/frontend/js/pages/ide.tsx @@ -0,0 +1,6 @@ +import '../utils/webpack-public-path' // configure dynamically loaded assets (via webpack) to be downloaded from CDN +import '../infrastructure/error-reporter' // set up error reporting, including Sentry +import ReactDOM from 'react-dom' +import IdeRoot from '@/features/ide-react/components/ide-root' + +ReactDOM.render(, document.getElementById('ide-root')) diff --git a/services/web/frontend/js/shared/hooks/use-wait-for-i18n.ts b/services/web/frontend/js/shared/hooks/use-wait-for-i18n.ts index 60f1144509..b81b5ce363 100644 --- a/services/web/frontend/js/shared/hooks/use-wait-for-i18n.ts +++ b/services/web/frontend/js/shared/hooks/use-wait-for-i18n.ts @@ -5,15 +5,21 @@ import { useTranslation } from 'react-i18next' function useWaitForI18n() { const { ready: isHookReady } = useTranslation() const [isLocaleDataLoaded, setIsLocaleDataLoaded] = useState(false) + const [error, setError] = useState() useEffect(() => { - i18n.then(() => { - setIsLocaleDataLoaded(true) - }) + i18n + .then(() => { + setIsLocaleDataLoaded(true) + }) + .catch(error => { + setError(error) + }) }, []) return { isReady: isHookReady && isLocaleDataLoaded, + error, } }