Jelajahi Sumber

[ide-react] Improve initial loading behaviour (#15916)

* Defer script loading
* Only mount IdePage once everything has connected

GitOrigin-RevId: 32f16214f26ac6a6d71a9dd332b3c35b8b82deae
Alf Eaton 2 tahun lalu
induk
melakukan
345f51bedb

+ 13 - 11
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";
+			}
+		})

+ 3 - 2
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)

+ 4 - 12
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 (
     <ReactContextRoot>
-      <Loading loadingText={loadingText}>
-        <IdePage />
-      </Loading>
+      {loaded ? <IdePage /> : <Loading setLoaded={setLoaded} />}
     </ReactContextRoot>
   )
 }

+ 48 - 48
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<LoadStatus, number> = {
-  initial: 20,
-  rendered: 40,
-  connected: 70,
-  loaded: 100,
-}
+const initialParts = new Set<Part>(['initial'])
 
-// 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<LoadStatus>('initial')
-  const { connectionState, isConnected } = useConnectionContext()
-  const loadProgress = loadProgressPercentage[loadStatus]
-  const editorLoaded = loadStatus === 'loaded'
+const totalParts = new Set<Part>([
+  'initial',
+  'render',
+  'connection',
+  'translations',
+  'project',
+])
 
-  const [i18nLoaded, setI18nLoaded] = useState(false)
-  const [translationLoadError, setTranslationLoadError] = useState(false)
+export const Loading: FC<{
+  setLoaded: (value: boolean) => void
+}> = ({ setLoaded }) => {
+  const [loadedParts, setLoadedParts] = useState(initialParts)
+
+  const progress = (loadedParts.size / totalParts.size) * 100
 
-  // 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')
-  }, [])
+    setLoaded(progress === 100)
+  }, [progress, setLoaded])
+
+  const { connectionState, isConnected } = useConnectionContext()
+  const i18n = useWaitForI18n()
+  const { projectJoined } = useIdeReactContext()
 
   useEffect(() => {
-    i18n
-      .then(() => setI18nLoaded(true))
-      .catch(() => {
-        setTranslationLoadError(true)
-      })
+    setLoadedParts(value => new Set(value).add('render'))
   }, [])
 
   useEffect(() => {
-    if (editorLoaded) {
-      return
-    }
     if (isConnected) {
-      setLoadStatus(i18nLoaded ? 'loaded' : 'connected')
+      setLoadedParts(value => new Set(value).add('connection'))
+    }
+  }, [isConnected])
+
+  useEffect(() => {
+    if (i18n.isReady) {
+      setLoadedParts(value => new Set(value).add('translations'))
     }
-  }, [i18nLoaded, editorLoaded, setLoadStatus, isConnected])
+  }, [i18n.isReady])
+
+  useEffect(() => {
+    if (projectJoined) {
+      setLoadedParts(value => new Set(value).add('project'))
+    }
+  }, [projectJoined])
+
+  const error =
+    connectionState.error ||
+    (i18n.error ? getMeta('ol-translationLoadErrorMessage') : '')
 
-  const translationLoadErrorMessage = translationLoadError
-    ? getMeta('ol-translationLoadErrorMessage')
-    : ''
+  // Use loading text from the server, because i18n will not be ready initially
+  const label = getMeta('ol-loadingText')
 
-  return editorLoaded ? (
-    <>{children}</>
-  ) : (
+  return (
     <div className="loading-screen">
-      <LoadingBranded
-        loadProgress={loadProgress}
-        label={loadingText}
-        error={connectionState.error || translationLoadErrorMessage}
-      />
+      <LoadingBranded loadProgress={progress} label={label} error={error} />
     </div>
   )
 }

+ 0 - 21
services/web/frontend/js/pages/ide.jsx

@@ -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(<IdeRoot />, element)
-}

+ 6 - 0
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(<IdeRoot />, document.getElementById('ide-root'))

+ 9 - 3
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<Error>()
 
   useEffect(() => {
-    i18n.then(() => {
-      setIsLocaleDataLoaded(true)
-    })
+    i18n
+      .then(() => {
+        setIsLocaleDataLoaded(true)
+      })
+      .catch(error => {
+        setError(error)
+      })
   }, [])
 
   return {
     isReady: isHookReady && isLocaleDataLoaded,
+    error,
   }
 }