diff --git a/src/alerts/access-expiration-alert/hooks.js b/src/alerts/access-expiration-alert/hooks.js index c60c159d..0aaa12a4 100644 --- a/src/alerts/access-expiration-alert/hooks.js +++ b/src/alerts/access-expiration-alert/hooks.js @@ -7,11 +7,9 @@ function useAccessExpirationAlert(courseExpiredMessage, topic) { const rawHtml = courseExpiredMessage || null; const isVisible = !!rawHtml; // If it exists, show it. - const payload = useMemo(() => ({ rawHtml }), [rawHtml]); - useAlert(isVisible, { code: 'clientAccessExpirationAlert', - payload, + payload: useMemo(() => ({ rawHtml }), [rawHtml]), topic, }); diff --git a/src/alerts/enrollment-alert/hooks.js b/src/alerts/enrollment-alert/hooks.js index 0f1e44a9..f981dace 100644 --- a/src/alerts/enrollment-alert/hooks.js +++ b/src/alerts/enrollment-alert/hooks.js @@ -1,6 +1,6 @@ /* eslint-disable import/prefer-default-export */ import React, { - useContext, useState, useCallback, + useContext, useState, useCallback, useMemo, } from 'react'; import { UserMessagesContext, ALERT_TYPES, useAlert } from '../../generic/user-messages'; @@ -14,15 +14,16 @@ export function useEnrollmentAlert(courseId) { const course = useModel('courses', courseId); const outline = useModel('outline', courseId); const isVisible = course && course.isEnrolled !== undefined && !course.isEnrolled; + const payload = { + canEnroll: outline.enrollAlert.canEnroll, + courseId, + extraText: outline.enrollAlert.extraText, + isStaff: course.isStaff, + }; useAlert(isVisible, { code: 'clientEnrollmentAlert', - payload: { - canEnroll: outline.enrollAlert.canEnroll, - courseId, - extraText: outline.enrollAlert.extraText, - isStaff: course.isStaff, - }, + payload: useMemo(() => payload, Object.values(payload).sort()), topic: 'outline', }); diff --git a/src/alerts/offer-alert/hooks.js b/src/alerts/offer-alert/hooks.js index b3020cb0..97f39509 100644 --- a/src/alerts/offer-alert/hooks.js +++ b/src/alerts/offer-alert/hooks.js @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useMemo } from 'react'; import { useAlert } from '../../generic/user-messages'; const OfferAlert = React.lazy(() => import('./OfferAlert')); @@ -10,7 +10,7 @@ export function useOfferAlert(offerHtml, topic) { useAlert(isVisible, { code: 'clientOfferAlert', topic, - payload: { rawHtml }, + payload: useMemo(() => ({ rawHtml }), [rawHtml]), }); return { clientOfferAlert: OfferAlert }; diff --git a/src/course-home/outline-tab/alerts/certificate-available-alert/hooks.js b/src/course-home/outline-tab/alerts/certificate-available-alert/hooks.js index a4ee0261..fce997c2 100644 --- a/src/course-home/outline-tab/alerts/certificate-available-alert/hooks.js +++ b/src/course-home/outline-tab/alerts/certificate-available-alert/hooks.js @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useMemo } from 'react'; import { getAuthenticatedUser } from '@edx/frontend-platform/auth'; import { useAlert } from '../../../../generic/user-messages'; @@ -23,14 +23,15 @@ function useCertificateAvailableAlert(courseId) { const endDate = endBlock ? new Date(endBlock.date) : null; const hasEnded = endBlock ? endDate < new Date() : false; const isVisible = isEnrolled && certBlock && hasEnded; // only show if we're between end and cert dates + const payload = { + certDate: certBlock && certBlock.date, + username, + userTimezone, + }; useAlert(isVisible, { code: 'clientCertificateAvailableAlert', - payload: { - certDate: certBlock && certBlock.date, - username, - userTimezone, - }, + payload: useMemo(() => payload, Object.values(payload).sort()), topic: 'outline-course-alerts', }); diff --git a/src/course-home/outline-tab/alerts/course-end-alert/hooks.js b/src/course-home/outline-tab/alerts/course-end-alert/hooks.js index 851087c0..09a8582f 100644 --- a/src/course-home/outline-tab/alerts/course-end-alert/hooks.js +++ b/src/course-home/outline-tab/alerts/course-end-alert/hooks.js @@ -1,5 +1,5 @@ /* eslint-disable import/prefer-default-export */ -import React from 'react'; +import React, { useMemo } from 'react'; import { useAlert } from '../../../../generic/user-messages'; import { useModel } from '../../../../generic/model-store'; @@ -23,15 +23,16 @@ export function useCourseEndAlert(courseId) { const endDate = endBlock ? new Date(endBlock.date) : null; const delta = endBlock ? endDate - new Date() : 0; const isVisible = isEnrolled && endBlock && delta > 0 && delta < WARNING_PERIOD_MS; + const payload = { + delta, + description: endBlock && endBlock.description, + endDate: endBlock && endBlock.date, + userTimezone, + }; useAlert(isVisible, { code: 'clientCourseEndAlert', - payload: { - delta, - description: endBlock && endBlock.description, - endDate: endBlock && endBlock.date, - userTimezone, - }, + payload: useMemo(() => payload, Object.values(payload).sort()), topic: 'outline-course-alerts', }); diff --git a/src/generic/user-messages/hooks.js b/src/generic/user-messages/hooks.js index 1f010874..97ce79b4 100644 --- a/src/generic/user-messages/hooks.js +++ b/src/generic/user-messages/hooks.js @@ -8,6 +8,17 @@ export function useAlert(isVisible, { const { add, remove } = useContext(UserMessagesContext); const [alertId, setAlertId] = useState(null); + // Please note: + // The deps list [isVisible, code, ... etc.] in this `useEffect` call prevents the + // effect from running if none of deps have changed. However, "changed" for objects is + // defined in terms of identity; thus, if you provide a payload that is *seemingly* equal + // to the previous one but *actually* a different object, then this effect will run. + // If you are particularly unlucky, this will cause an infinite re-render loop. + // This manifested itself in TNL-7400. + // We hope to address the underlying issue in TNL-7418. + // In the mean time, you may follow the pattern that `useAccessExpirationAlert` + // establishes: memoize the payload so that the exact same object is used if the + // payload has not changed. useEffect(() => { if (isVisible && alertId === null) { setAlertId(add({