Use layout effect to avoid iframe pausing React lifecycle (#56)
Fixes TNL-7187 - Adds a no-op useLayoutEffect hook to Unit.jsx to prevent the unit iframe from pausing React’s rendering lifecycle. Very strange bug - see comments in that file for more detail.
This commit is contained in:
@@ -1,8 +1,8 @@
|
||||
import React, {
|
||||
Suspense,
|
||||
useRef,
|
||||
useEffect,
|
||||
useState,
|
||||
useLayoutEffect,
|
||||
} from 'react';
|
||||
import PropTypes from 'prop-types';
|
||||
import { getConfig } from '@edx/frontend-platform';
|
||||
@@ -14,13 +14,44 @@ import PageLoading from '../../../PageLoading';
|
||||
|
||||
const LockPaywall = React.lazy(() => import('./lock-paywall'));
|
||||
|
||||
/**
|
||||
* We discovered an error in Firefox where - upon iframe load - React would cease to call any
|
||||
* useEffect hooks until the user interacts with the page again. This is particularly confusing
|
||||
* when navigating between sequences, as the UI partially updates leaving the user in a nebulous
|
||||
* state.
|
||||
*
|
||||
* We were able to solve this error by using a layout effect to update some component state, which
|
||||
* executes synchronously on render. Somehow this forces React to continue it's lifecycle
|
||||
* immediately, rather than waiting for user interaction. This layout effect could be anywhere in
|
||||
* the parent tree, as far as we can tell - we chose to add a conspicuously 'load bearing' (that's
|
||||
* a joke) one here so it wouldn't be accidentally removed elsewhere.
|
||||
*
|
||||
* If we remove this hook when one of these happens:
|
||||
* 1. React figures out that there's an issue here and fixes a bug.
|
||||
* 2. We cease to use an iframe for unit rendering.
|
||||
* 3. Firefox figures out that there's an issue in their iframe loading and fixes a bug.
|
||||
* 4. We stop supporting Firefox.
|
||||
* 5. An enterprising engineer decides to create a repo that reproduces the problem, submits it to
|
||||
* Firefox/React for review, and they kindly help us figure out what in the world is happening
|
||||
* so we can fix it.
|
||||
*
|
||||
* This hook depends on the unit id just to make sure it re-evaluates whenever the ID changes. If
|
||||
* we change whether or not the Unit component is re-mounted when the unit ID changes, this may
|
||||
* become important, as this hook will otherwise only evaluate the useLayoutEffect once.
|
||||
*/
|
||||
function useLoadBearingHook(id) {
|
||||
const setValue = useState(0)[1];
|
||||
useLayoutEffect(() => {
|
||||
setValue(currentValue => currentValue + 1);
|
||||
}, [id]);
|
||||
}
|
||||
|
||||
function Unit({
|
||||
courseId,
|
||||
onLoaded,
|
||||
id,
|
||||
intl,
|
||||
}) {
|
||||
const iframeRef = useRef(null);
|
||||
const iframeUrl = `${getConfig().LMS_BASE_URL}/xblock/${id}?show_title=0&show_bookmark_button=0`;
|
||||
|
||||
const [iframeHeight, setIframeHeight] = useState(0);
|
||||
@@ -33,25 +64,29 @@ function Unit({
|
||||
enrollmentMode,
|
||||
} = course;
|
||||
|
||||
useEffect(() => {
|
||||
global.onmessage = (event) => {
|
||||
const { type, payload } = event.data;
|
||||
// Do not remove this hook. See function description.
|
||||
useLoadBearingHook(id);
|
||||
|
||||
console.log('MFE App', 'onmessage', id, iframeUrl, event.source, type, payload, event);
|
||||
if (type === 'plugin.resize') {
|
||||
setIframeHeight(payload.height);
|
||||
console.log('MFE App', 'plugin.resize', hasLoaded, iframeHeight, payload.height);
|
||||
if (!hasLoaded && iframeHeight === 0 && payload.height > 0) {
|
||||
console.log('MFE App', 'hasLoaded', true);
|
||||
setHasLoaded(true);
|
||||
if (onLoaded) {
|
||||
console.log('MFE App', 'fire onLoaded handler');
|
||||
onLoaded();
|
||||
useEffect(() => {
|
||||
if (!global.onmessage) {
|
||||
global.onmessage = (event) => {
|
||||
const { type, payload } = event.data;
|
||||
|
||||
if (type === 'plugin.resize') {
|
||||
setIframeHeight(payload.height);
|
||||
if (!hasLoaded && iframeHeight === 0 && payload.height > 0) {
|
||||
setHasLoaded(true);
|
||||
if (onLoaded) {
|
||||
onLoaded();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
};
|
||||
}
|
||||
return () => {
|
||||
global.onmessage = null;
|
||||
};
|
||||
}, []);
|
||||
}, [id]);
|
||||
|
||||
return (
|
||||
<div className="unit">
|
||||
@@ -78,7 +113,6 @@ function Unit({
|
||||
<iframe
|
||||
id="unit-iframe"
|
||||
title={unit.title}
|
||||
ref={iframeRef}
|
||||
src={iframeUrl}
|
||||
allowFullScreen
|
||||
height={iframeHeight}
|
||||
|
||||
Reference in New Issue
Block a user