From b3a46a77914e6491554f8a436ae74ccea94ff76b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicol=C3=B2=20Pretto?= <info@npretto.com>
Date: Mon, 18 Nov 2024 16:27:25 +0100
Subject: [PATCH] refactor(sdk): use ignore flag instead of mounted ref
(#50069)
* Revert "fix(sdk): put a bandage on the flashing error on static question in strict mode (#49659)"
This reverts commit c57b3a38e0ec25f13127652b74751837d908f664.
* refactor(sdk): use ignore flag instead of mounted ref
---
.../hooks/private/use-load-static-question.ts | 45 ++++++++-----------
1 file changed, 18 insertions(+), 27 deletions(-)
diff --git a/enterprise/frontend/src/embedding-sdk/hooks/private/use-load-static-question.ts b/enterprise/frontend/src/embedding-sdk/hooks/private/use-load-static-question.ts
index f288b67d46b..d88795f431f 100644
--- a/enterprise/frontend/src/embedding-sdk/hooks/private/use-load-static-question.ts
+++ b/enterprise/frontend/src/embedding-sdk/hooks/private/use-load-static-question.ts
@@ -1,4 +1,4 @@
-import { useEffect, useRef, useState } from "react";
+import { useEffect, useState } from "react";
import { loadStaticQuestion } from "embedding-sdk/lib/load-static-question";
import type { GenericErrorResponse } from "metabase/lib/errors";
@@ -17,17 +17,6 @@ export function useLoadStaticQuestion(
questionId: number | null,
parameterValues?: Record<string, string | number>,
) {
- // This is a hack around strict mode calling the useEffect twice ->
- // the first request is being cancelled and we render a error state for a few renders
- // This needs to start at true, it needs to bypass the double use effect of strict mode
- // See https://github.com/metabase/metabase/issues/49620 for more details on the issue
- const isMounted = useRef(true);
- useEffect(() => {
- return () => {
- isMounted.current = false;
- };
- }, []);
-
const [questionState, setQuestionState] = useState<QuestionState>({
loading: false,
card: null,
@@ -45,6 +34,7 @@ export function useLoadStaticQuestion(
useEffect(() => {
const cancelDeferred = defer();
+ let ignore = false; // flag to ignore the result if the component unmounts: https://react.dev/learn/you-might-not-need-an-effect#fetching-data
async function loadCardData() {
setQuestionState(state => ({ ...state, loading: true }));
@@ -60,24 +50,24 @@ export function useLoadStaticQuestion(
cancelDeferred,
});
- setQuestionState({
- card,
- result,
- loading: false,
- error: null,
- });
- } catch (error) {
- if (!isMounted.current) {
- return;
- }
-
- if (typeof error === "object") {
+ if (!ignore) {
setQuestionState({
- result: null,
- card: null,
+ card,
+ result,
loading: false,
- error,
+ error: null,
});
+ }
+ } catch (error) {
+ if (typeof error === "object") {
+ if (!ignore) {
+ setQuestionState({
+ result: null,
+ card: null,
+ loading: false,
+ error,
+ });
+ }
} else {
console.error("error loading static question", error);
}
@@ -89,6 +79,7 @@ export function useLoadStaticQuestion(
return () => {
// cancel pending requests upon unmount
cancelDeferred.resolve();
+ ignore = true;
};
}, [questionId, parameterValues]);
--
GitLab