This is an automated email from the ASF dual-hosted git repository.
bbovenzi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new fdf4adce2a3 UI: Use RadioCardRoot's typed onValueChange instead of DOM
casts (#73985)
fdf4adce2a3 is described below
commit fdf4adce2a3e04ad11f6ab262fcb36fb0ac36c42
Author: Brent Bovenzi <[email protected]>
AuthorDate: Tue Oct 13 14:51:06 2026 -0400
UI: Use RadioCardRoot's typed onValueChange instead of DOM casts (#73985)
Reading the selected value off a raw change event required casting
event.target to HTMLInputElement, and for the enum/union-typed call
sites a second `as` cast on top of that. Those casts are unchecked: a
radio value renamed in the markup but not in the type still compiles
and only breaks at runtime. Chakra v3's RadioCard.Root already hands
the value to onValueChange, so narrowing can be done with a type
predicate and the compiler verifies the value set matches the type.
The predicate was being hand-written per call site, so it moves to
src/utils/isOneOf.ts and narrows to the literal union of the values it
is given. Where a generated API type already describes that union the
values are checked against it, rather than a hand-maintained copy.
Two CreateAssetEventModal tests had to await the submit button: the
value now arrives through Ark UI's state machine, so the re-render no
longer lands synchronously inside fireEvent.
---
.../src/components/TriggerDag/TriggerDAGModal.tsx | 9 ++++++--
.../src/pages/Asset/CreateAssetEventModal.test.tsx | 24 +++++++++----------
.../ui/src/pages/Asset/CreateAssetEventModal.tsx | 12 ++++++----
.../ui/src/pages/Dag/Backfills/Backfills.tsx | 9 +++-----
.../ui/src/pages/Variables/ImportVariablesForm.tsx | 21 +++++++++++------
airflow-core/src/airflow/ui/src/utils/index.ts | 1 +
.../ui/src/utils/{index.ts => isOneOf.test.ts} | 27 +++++++++++-----------
.../airflow/ui/src/utils/{index.ts => isOneOf.ts} | 22 ++++++++----------
8 files changed, 68 insertions(+), 57 deletions(-)
diff --git
a/airflow-core/src/airflow/ui/src/components/TriggerDag/TriggerDAGModal.tsx
b/airflow-core/src/airflow/ui/src/components/TriggerDag/TriggerDAGModal.tsx
index fc539337a19..047cdc68c99 100644
--- a/airflow-core/src/airflow/ui/src/components/TriggerDag/TriggerDAGModal.tsx
+++ b/airflow-core/src/airflow/ui/src/components/TriggerDag/TriggerDAGModal.tsx
@@ -26,6 +26,7 @@ import { useDagServiceGetDag } from "openapi/queries";
import { Modal, Tooltip, RadioCardItem, RadioCardRoot } from
"src/system-components";
import { useTrigger } from "src/queries/useTrigger";
+import { isOneOf } from "src/utils";
import RunBackfillForm from "../DagActions/RunBackfillForm";
import TriggerDAGForm from "./TriggerDAGForm";
@@ -35,6 +36,8 @@ enum RunMode {
SINGLE = "single",
}
+const isRunMode = isOneOf(Object.values(RunMode));
+
type TriggerDAGModalProps = {
readonly dagDisplayName: string;
readonly dagId: string;
@@ -106,8 +109,10 @@ const TriggerDAGModal = ({ dagDisplayName, dagId, onClose,
open, prefillConfig }
{dag ? (
<RadioCardRoot
my={4}
- onChange={(event) => {
- setRunMode((event.target as HTMLInputElement).value as
RunMode);
+ onValueChange={(details) => {
+ if (isRunMode(details.value)) {
+ setRunMode(details.value);
+ }
}}
value={runMode}
>
diff --git
a/airflow-core/src/airflow/ui/src/pages/Asset/CreateAssetEventModal.test.tsx
b/airflow-core/src/airflow/ui/src/pages/Asset/CreateAssetEventModal.test.tsx
index 855f0ce2c38..37bcb5d91da 100644
--- a/airflow-core/src/airflow/ui/src/pages/Asset/CreateAssetEventModal.test.tsx
+++ b/airflow-core/src/airflow/ui/src/pages/Asset/CreateAssetEventModal.test.tsx
@@ -216,8 +216,8 @@ describe("CreateAssetEventModal", () => {
{ wrapper: Wrapper },
);
fireEvent.click(screen.getByText("createEvent.materialize.label"));
- expect(screen.getByText("submit materialize")).toBeDisabled();
- fireEvent.click(screen.getByText("submit materialize"));
+ expect(await screen.findByText("submit materialize")).toBeDisabled();
+ fireEvent.click(await screen.findByText("submit materialize"));
expect(materializeAsset).not.toHaveBeenCalled();
act(() => {
resolveDag?.({ ...upstreamDag, is_paused: !cachedPaused });
@@ -228,7 +228,7 @@ describe("CreateAssetEventModal", () => {
},
);
- it("blocks materialization after the Dag refresh fails", () => {
+ it("blocks materialization after the Dag refresh fails", async () => {
vi.mocked(useDependenciesServiceGetDependencies).mockReturnValue(withUpstreamDependencies);
vi.mocked(useDagServiceGetDagDetails).mockReturnValue({
data: upstreamDag,
@@ -237,10 +237,10 @@ describe("CreateAssetEventModal", () => {
} as ReturnType<typeof useDagServiceGetDagDetails>);
render(<CreateAssetEventModal asset={asset} onClose={vi.fn()} open />, {
wrapper: Wrapper });
fireEvent.click(screen.getByText("createEvent.materialize.label"));
- expect(screen.getByText("submit materialize")).toBeDisabled();
+ expect(await screen.findByText("submit materialize")).toBeDisabled();
});
- it("resets a materialization error when the paused Dag choice changes", ()
=> {
+ it("resets a materialization error when the paused Dag choice changes",
async () => {
vi.mocked(useAssetServiceMaterializeAsset).mockReturnValue({
error: { status: 403 },
isPending: false,
@@ -253,7 +253,7 @@ describe("CreateAssetEventModal", () => {
} as ReturnType<typeof useDagServiceGetDagDetails>);
render(<CreateAssetEventModal asset={asset} onClose={vi.fn()} open />, {
wrapper: Wrapper });
fireEvent.click(screen.getByText("createEvent.materialize.label"));
- fireEvent.click(screen.getByText("Keep paused"));
+ fireEvent.click(await screen.findByText("Keep paused"));
expect(resetMaterializeError).toHaveBeenCalledOnce();
});
@@ -302,7 +302,7 @@ describe("CreateAssetEventModal", () => {
});
});
- it("sends materialize partition_key as null when the trigger form leaves it
undefined", () => {
+ it("sends materialize partition_key as null when the trigger form leaves it
undefined", async () => {
vi.mocked(useDependenciesServiceGetDependencies).mockReturnValue(withUpstreamDependencies);
vi.mocked(useDagServiceGetDagDetails).mockReturnValue({
data: upstreamDag,
@@ -312,7 +312,7 @@ describe("CreateAssetEventModal", () => {
render(<CreateAssetEventModal asset={asset} onClose={vi.fn()} open />, {
wrapper: Wrapper });
fireEvent.click(screen.getByText("createEvent.materialize.label"));
- fireEvent.click(screen.getByText("submit materialize"));
+ fireEvent.click(await screen.findByText("submit materialize"));
expect(materializeAsset).toHaveBeenCalledWith(
expect.objectContaining({
@@ -321,7 +321,7 @@ describe("CreateAssetEventModal", () => {
);
});
- it("forwards the trigger form's drain choice as drain_dag", () => {
+ it("forwards the trigger form's drain choice as drain_dag", async () => {
vi.mocked(useDependenciesServiceGetDependencies).mockReturnValue(withUpstreamDependencies);
vi.mocked(useDagServiceGetDagDetails).mockReturnValue({
data: upstreamDag,
@@ -331,7 +331,7 @@ describe("CreateAssetEventModal", () => {
render(<CreateAssetEventModal asset={asset} onClose={vi.fn()} open />, {
wrapper: Wrapper });
fireEvent.click(screen.getByText("createEvent.materialize.label"));
- fireEvent.click(screen.getByText("submit materialize"));
+ fireEvent.click(await screen.findByText("submit materialize"));
expect(materializeAsset).toHaveBeenCalledWith(
expect.objectContaining({
@@ -371,7 +371,7 @@ describe("CreateAssetEventModal", () => {
}
});
- it("sends the materialize partition_key from the trigger form as-is", () => {
+ it("sends the materialize partition_key from the trigger form as-is", async
() => {
vi.mocked(useDependenciesServiceGetDependencies).mockReturnValue(withUpstreamDependencies);
vi.mocked(useDagServiceGetDagDetails).mockReturnValue({
data: upstreamDag,
@@ -381,7 +381,7 @@ describe("CreateAssetEventModal", () => {
render(<CreateAssetEventModal asset={asset} onClose={vi.fn()} open />, {
wrapper: Wrapper });
fireEvent.click(screen.getByText("createEvent.materialize.label"));
- fireEvent.click(screen.getByText("submit materialize"));
+ fireEvent.click(await screen.findByText("submit materialize"));
expect(materializeAsset).toHaveBeenCalledWith(
expect.objectContaining({
diff --git
a/airflow-core/src/airflow/ui/src/pages/Asset/CreateAssetEventModal.tsx
b/airflow-core/src/airflow/ui/src/pages/Asset/CreateAssetEventModal.tsx
index 4a9e8359ee8..26a8366b29a 100644
--- a/airflow-core/src/airflow/ui/src/pages/Asset/CreateAssetEventModal.tsx
+++ b/airflow-core/src/airflow/ui/src/pages/Asset/CreateAssetEventModal.tsx
@@ -51,7 +51,9 @@ import { JsonEditor } from "src/components/JsonEditor";
import TriggerDAGForm from "src/components/TriggerDag/TriggerDAGForm";
import type { DagRunTriggerParams } from "src/components/TriggerDag/types";
-import { toNullablePartitionKey } from "src/utils";
+import { isOneOf, toNullablePartitionKey } from "src/utils";
+
+const isAssetEventType = isOneOf(["manual", "materialize"] as const);
type Props = {
readonly asset: AssetResponse;
@@ -61,7 +63,7 @@ type Props = {
export const CreateAssetEventModal = ({ asset, onClose, open }: Props) => {
const { t: translate } = useTranslation(["assets", "components"]);
- const [eventType, setEventType] = useState("manual");
+ const [eventType, setEventType] = useState<"manual" |
"materialize">("manual");
const [extraError, setExtraError] = useState<string | undefined>();
const [extra, setExtra] = useState("{}");
const [partitionKey, setPartitionKey] = useState<string |
undefined>(undefined);
@@ -197,8 +199,10 @@ export const CreateAssetEventModal = ({ asset, onClose,
open }: Props) => {
>
<RadioCardRoot
mb={6}
- onChange={(event) => {
- setEventType((event.target as HTMLInputElement).value);
+ onValueChange={(details) => {
+ if (isAssetEventType(details.value)) {
+ setEventType(details.value);
+ }
}}
value={eventType}
>
diff --git a/airflow-core/src/airflow/ui/src/pages/Dag/Backfills/Backfills.tsx
b/airflow-core/src/airflow/ui/src/pages/Dag/Backfills/Backfills.tsx
index af18360831a..398394ebee2 100644
--- a/airflow-core/src/airflow/ui/src/pages/Dag/Backfills/Backfills.tsx
+++ b/airflow-core/src/airflow/ui/src/pages/Dag/Backfills/Backfills.tsx
@@ -32,7 +32,7 @@ import { ErrorAlert } from "src/components/ErrorAlert";
import Time from "src/components/Time";
import { SearchParamsKeys, type SearchParamsKeysType } from
"src/constants/searchParams";
-import { type DurationFormat, useDurationFormat } from "src/utils";
+import { type DurationFormat, isOneOf, useDurationFormat } from "src/utils";
import { BackfillDagRunsModal } from "./BackfillDagRunsModal";
import { BackfillsFilters } from "./BackfillsFilters";
@@ -53,14 +53,11 @@ const {
TO_DATE_LTE: TO_DATE_LTE_PARAM,
}: SearchParamsKeysType = SearchParamsKeys;
-const REPROCESS_BEHAVIOR_VALUES = [
+const isReprocessBehavior = isOneOf([
"failed",
"completed",
"none",
-] as const satisfies ReadonlyArray<ReprocessBehavior>;
-
-const isReprocessBehavior = (value: string | null): value is ReprocessBehavior
=>
- (REPROCESS_BEHAVIOR_VALUES as ReadonlyArray<string | null>).includes(value);
+] as const satisfies ReadonlyArray<ReprocessBehavior>);
type ColumnProps = {
readonly onSelectBackfill: (backfillId: number) => void;
diff --git
a/airflow-core/src/airflow/ui/src/pages/Variables/ImportVariablesForm.tsx
b/airflow-core/src/airflow/ui/src/pages/Variables/ImportVariablesForm.tsx
index 9c2ecd77eb0..abaa8393425 100644
--- a/airflow-core/src/airflow/ui/src/pages/Variables/ImportVariablesForm.tsx
+++ b/airflow-core/src/airflow/ui/src/pages/Variables/ImportVariablesForm.tsx
@@ -24,13 +24,20 @@ import { useTranslation } from "react-i18next";
import { FiUploadCloud } from "react-icons/fi";
import { LuFileUp } from "react-icons/lu";
-import type { BulkBody_VariableBody_ } from "openapi/requests/types.gen";
+import type { BulkActionOnExistence, BulkBody_VariableBody_ } from
"openapi/requests/types.gen";
import { RadioCardItem, RadioCardLabel, RadioCardRoot } from
"src/system-components";
import { ErrorAlert } from "src/components/ErrorAlert";
import { useImportVariables } from "src/queries/useImportVariables";
+import { isOneOf } from "src/utils";
+
+const isActionOnExistence = isOneOf([
+ "fail",
+ "overwrite",
+ "skip",
+] as const satisfies ReadonlyArray<BulkActionOnExistence>);
type ImportVariablesFormProps = {
readonly onClose: () => void;
@@ -60,7 +67,7 @@ const ImportVariablesForm = ({ onClose }:
ImportVariablesFormProps) => {
onSuccessConfirm: onClose,
});
- const [actionIfExists, setActionIfExists] = useState<"fail" | "overwrite" |
"skip">("fail");
+ const [actionIfExists, setActionIfExists] =
useState<BulkActionOnExistence>("fail");
const [isParsing, setIsParsing] = useState(false);
const [fileContent, setFileContent] = useState<Record<string, unknown> |
undefined>(undefined);
@@ -166,13 +173,13 @@ const ImportVariablesForm = ({ onClose }:
ImportVariablesFormProps) => {
) : undefined}
</FileUpload.Root>
<RadioCardRoot
- defaultValue="fail"
mb={6}
- onChange={(event) => {
- const target = event.target as HTMLInputElement;
-
- setActionIfExists(target.value as "fail" | "overwrite" | "skip");
+ onValueChange={(details) => {
+ if (isActionOnExistence(details.value)) {
+ setActionIfExists(details.value);
+ }
}}
+ value={actionIfExists}
>
<RadioCardLabel fontSize="md" mb={3}>
{translate("variables.import.conflictResolution")}
diff --git a/airflow-core/src/airflow/ui/src/utils/index.ts
b/airflow-core/src/airflow/ui/src/utils/index.ts
index 84d5097eea8..e16659d78a8 100644
--- a/airflow-core/src/airflow/ui/src/utils/index.ts
+++ b/airflow-core/src/airflow/ui/src/utils/index.ts
@@ -22,6 +22,7 @@ export { createErrorToaster, getErrorStatus } from
"./errorHandling";
export { formatNumber } from "./formatNumber";
export { getMetaKey } from "./getMetaKey";
export { hasDagRunConfig } from "./hasDagRunConfig";
+export { isOneOf } from "./isOneOf";
export { toNullablePartitionKey } from "./partitionKey";
export { useContainerWidth } from "./useContainerWidth";
export { useDocumentTitle } from "./useDocumentTitle";
diff --git a/airflow-core/src/airflow/ui/src/utils/index.ts
b/airflow-core/src/airflow/ui/src/utils/isOneOf.test.ts
similarity index 51%
copy from airflow-core/src/airflow/ui/src/utils/index.ts
copy to airflow-core/src/airflow/ui/src/utils/isOneOf.test.ts
index 84d5097eea8..cc188928099 100644
--- a/airflow-core/src/airflow/ui/src/utils/index.ts
+++ b/airflow-core/src/airflow/ui/src/utils/isOneOf.test.ts
@@ -16,17 +16,18 @@
* specific language governing permissions and limitations
* under the License.
*/
+import { describe, expect, it } from "vitest";
-export { capitalize } from "./capitalize";
-export { createErrorToaster, getErrorStatus } from "./errorHandling";
-export { formatNumber } from "./formatNumber";
-export { getMetaKey } from "./getMetaKey";
-export { hasDagRunConfig } from "./hasDagRunConfig";
-export { toNullablePartitionKey } from "./partitionKey";
-export { useContainerWidth } from "./useContainerWidth";
-export { useDocumentTitle } from "./useDocumentTitle";
-export { type DurationFormat, useDurationFormat } from "./useDurationFormat";
-export { DocumentTitleProvider } from "./useDocumentTitleProvider";
-export { useFiltersHandler, type FilterableSearchParamsKeys } from
"./useFiltersHandler";
-export * from "./query";
-export { STATE_PRIORITY, sortStateEntries } from "./stateUtils";
+import { isOneOf } from "./isOneOf";
+
+const isColour = isOneOf(["red", "green"] as const);
+
+describe("isOneOf", () => {
+ it.each(["red", "green"])("accepts the allowed value %s", (value) => {
+ expect(isColour(value)).toBe(true);
+ });
+
+ it.each([["blue"], ["RED"], [""], [null]])("rejects %s", (value) => {
+ expect(isColour(value)).toBe(false);
+ });
+});
diff --git a/airflow-core/src/airflow/ui/src/utils/index.ts
b/airflow-core/src/airflow/ui/src/utils/isOneOf.ts
similarity index 51%
copy from airflow-core/src/airflow/ui/src/utils/index.ts
copy to airflow-core/src/airflow/ui/src/utils/isOneOf.ts
index 84d5097eea8..2983ee4fff1 100644
--- a/airflow-core/src/airflow/ui/src/utils/index.ts
+++ b/airflow-core/src/airflow/ui/src/utils/isOneOf.ts
@@ -17,16 +17,12 @@
* under the License.
*/
-export { capitalize } from "./capitalize";
-export { createErrorToaster, getErrorStatus } from "./errorHandling";
-export { formatNumber } from "./formatNumber";
-export { getMetaKey } from "./getMetaKey";
-export { hasDagRunConfig } from "./hasDagRunConfig";
-export { toNullablePartitionKey } from "./partitionKey";
-export { useContainerWidth } from "./useContainerWidth";
-export { useDocumentTitle } from "./useDocumentTitle";
-export { type DurationFormat, useDurationFormat } from "./useDurationFormat";
-export { DocumentTitleProvider } from "./useDocumentTitleProvider";
-export { useFiltersHandler, type FilterableSearchParamsKeys } from
"./useFiltersHandler";
-export * from "./query";
-export { STATE_PRIORITY, sortStateEntries } from "./stateUtils";
+/**
+ * Builds a type predicate from the allowed values, narrowing to their literal
+ * union. Pair with `satisfies ReadonlyArray<ApiType>` at the call site to keep
+ * the values tied to a generated API type.
+ */
+export const isOneOf =
+ <const T extends ReadonlyArray<string>>(values: T) =>
+ (value: string | null): value is T[number] =>
+ (values as ReadonlyArray<string | null>).includes(value);