Skip to content

Commit c8f15b3

Browse files
authored
fix: inconsistent behavior between ImportComponent and Drop on canvas (#1487)
## Description Closes Shopify/oasis-frontend#226 Refactored component import functionality to use React Query mutations and improved event handling. The main changes include: - Made `ComponentDuplicateDialog` wait for the import component operation to complete by adding `await` - Replaced the deprecated `useImportComponent` hook with direct usage of the `ComponentLibraryProvider` - Added custom DOM event handling for component library events - Implemented loading state indicators with a spinner during component import - Added event dispatching when components are added to the library - Added tests for the new event-based functionality ## Type of Change - [x] Improvement - [x] Cleanup/Refactor ## Checklist - [x] I have tested this does not break current pipelines / runs functionality - [ ] I have tested the changes on staging ## Test Instructions 1. [Screen Recording 2025-12-06 at 12.18.30 PM.mov <span class="graphite__hidden">(uploaded via Graphite)</span> <img class="graphite__hidden" src="https://app.graphite.com/user-attachments/thumbnails/4afe34ee-da7b-46e2-9134-af5f70655cc2.mov" />](https://app.graphite.com/user-attachments/video/4afe34ee-da7b-46e2-9134-af5f70655cc2.mov) [Screen Recording 2025-12-06 at 12.15.30 PM.mov <span class="graphite__hidden">(uploaded via Graphite)</span> <img class="graphite__hidden" src="https://app.graphite.com/user-attachments/thumbnails/f6f138a6-e1bf-43fb-bdd9-936a1db085fc.mov" />](https://app.graphite.com/user-attachments/video/f6f138a6-e1bf-43fb-bdd9-936a1db085fc.mov) Import a component using the import dialog 2. Verify the loading state is displayed correctly 3. Confirm the component is successfully added to the library 4. Check that duplicate component handling works as expected 5. Test that all 3 major ways of importing works exactly same 1. Test import via "Add component" from "Used in Pipelines" 2. Test import via "Drop on canvas" 3. Test import from "Import Dialog"
1 parent 861c8e9 commit c8f15b3

File tree

7 files changed

+177
-47
lines changed

7 files changed

+177
-47
lines changed

src/components/shared/Dialogs/ComponentDuplicateDialog.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ const ComponentDuplicateDialog = ({
119119
USER_COMPONENTS_LIST_NAME,
120120
existingComponent?.name ?? "",
121121
);
122-
handleImportComponent(yamlString);
122+
await handleImportComponent(yamlString);
123123

124124
setClose();
125125
}, [handleImportComponent, setClose]);

src/components/shared/ReactFlow/FlowSidebar/components/ImportComponent.tsx

Lines changed: 92 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { useMutation } from "@tanstack/react-query";
12
import { PackagePlus, X } from "lucide-react";
23
import { Upload } from "lucide-react";
34
import {
@@ -25,10 +26,15 @@ import {
2526
} from "@/components/ui/dialog";
2627
import { Input } from "@/components/ui/input";
2728
import { Label } from "@/components/ui/label";
29+
import { Spinner } from "@/components/ui/spinner";
2830
import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs";
29-
import useImportComponent from "@/hooks/useImportComponent";
3031
import useToastNotification from "@/hooks/useToastNotification";
3132
import { cn } from "@/lib/utils";
33+
import { useComponentLibrary } from "@/providers/ComponentLibraryProvider";
34+
import {
35+
getStringFromData,
36+
hydrateComponentReference,
37+
} from "@/services/componentService";
3238

3339
enum TabType {
3440
URL = "URL",
@@ -42,13 +48,13 @@ const ImportComponent = ({
4248
triggerComponent?: ReactNode;
4349
}) => {
4450
const notify = useToastNotification();
51+
const { addToComponentLibrary } = useComponentLibrary();
4552

4653
const hasEnabledInAppEditor = useBetaFlagValue("in-app-component-editor");
4754

4855
const [url, setUrl] = useState("");
4956
const [tab, setTab] = useState<TabType>(TabType.File);
5057
const [isOpen, setIsOpen] = useState(false);
51-
const [isSubmitting, setIsSubmitting] = useState(false);
5258
const [selectedFile, setSelectedFile] = useState<string | ArrayBuffer | null>(
5359
null,
5460
);
@@ -63,21 +69,6 @@ const ImportComponent = ({
6369
setIsOpen(false);
6470
};
6571

66-
const { onImportFromUrl, onImportFromFile, isLoading } = useImportComponent({
67-
successCallback: () => {
68-
setIsOpen(false);
69-
setUrl("");
70-
setSelectedFile(null);
71-
setSelectedFileName("");
72-
setIsSubmitting(false);
73-
notify("Component imported successfully", "success");
74-
},
75-
errorCallback: (error: Error) => {
76-
notify(error.message, "error");
77-
setIsSubmitting(false);
78-
},
79-
});
80-
8172
const handleTabChange = useCallback((value: TabType) => {
8273
setTab(value);
8374
}, []);
@@ -114,14 +105,60 @@ const ImportComponent = ({
114105
}
115106
}, []);
116107

108+
const { mutate: importComponent, isPending } = useMutation({
109+
mutationFn: async () => {
110+
const componentRef = {
111+
text:
112+
tab === TabType.File && selectedFile
113+
? getStringFromData(selectedFile)
114+
: undefined,
115+
url: tab === TabType.URL ? url : undefined,
116+
};
117+
118+
const hydratedComponent = await hydrateComponentReference(componentRef);
119+
120+
if (!hydratedComponent) {
121+
throw new Error("Failed to hydrate component");
122+
}
123+
124+
const abortController = new AbortController();
125+
126+
return await Promise.all([
127+
Promise.race([
128+
createPromiseFromDomEvent(
129+
window,
130+
"tangle.library.componentAdded",
131+
abortController.signal,
132+
),
133+
createPromiseFromDomEvent(
134+
window,
135+
"tangle.library.duplicateDialogClosed",
136+
abortController.signal,
137+
),
138+
]),
139+
addToComponentLibrary(hydratedComponent),
140+
]).finally(() => {
141+
abortController.abort();
142+
});
143+
},
144+
onSuccess: async ([result, _]) => {
145+
setIsOpen(false);
146+
setUrl("");
147+
setSelectedFile(null);
148+
setSelectedFileName("");
149+
150+
if (result instanceof CustomEvent && result.detail?.component) {
151+
notify("Component imported successfully", "success");
152+
}
153+
},
154+
onError: (error: Error) => {
155+
notify(error.message, "error");
156+
},
157+
});
158+
117159
const handleImport = useCallback(() => {
118-
setIsSubmitting(true);
119-
if (tab === TabType.URL) {
120-
onImportFromUrl(url);
121-
} else if (tab === TabType.File && selectedFile) {
122-
onImportFromFile(selectedFile as string);
123-
}
124-
}, [tab, url, selectedFile]);
160+
void importComponent();
161+
}, [importComponent]);
125162

126163
const handleUrlChange = useCallback(
127164
(event: ChangeEvent<HTMLInputElement>) => {
@@ -132,22 +169,20 @@ const ImportComponent = ({
132169

133170
const handleOpenChange = useCallback(
134171
(open: boolean) => {
135-
if (!open && (isLoading || isSubmitting)) return;
172+
if (!open && isPending) return;
136173

137174
if (!open) {
138175
setUrl("");
139176
setSelectedFile(null);
140177
setSelectedFileName("");
141-
setIsSubmitting(false);
142178
}
143179
setIsOpen(open);
144180
},
145-
[isLoading, isSubmitting],
181+
[isPending],
146182
);
147183

148184
const isButtonDisabled =
149-
isLoading ||
150-
isSubmitting ||
185+
isPending ||
151186
(tab === TabType.URL && !url) ||
152187
(tab === TabType.File && !selectedFile);
153188

@@ -203,7 +238,7 @@ const ImportComponent = ({
203238
type="file"
204239
accept=".yaml"
205240
onChange={handleFileChange}
206-
disabled={isLoading || isSubmitting}
241+
disabled={isPending}
207242
ref={fileInputRef}
208243
className={`absolute inset-0 w-full h-full opacity-0 cursor-pointer ${selectedFileName ? "hidden" : ""}`}
209244
/>
@@ -263,7 +298,7 @@ const ImportComponent = ({
263298
placeholder="https://raw.githubusercontent.com/.../component.yaml"
264299
value={url}
265300
onChange={handleUrlChange}
266-
disabled={isLoading || isSubmitting}
301+
disabled={isPending}
267302
/>
268303
<p className="text-sm text-gray-500">
269304
Enter the URL of a component YAML file
@@ -293,7 +328,13 @@ const ImportComponent = ({
293328
onClick={handleImport}
294329
disabled={isButtonDisabled}
295330
>
296-
Import
331+
{isPending ? (
332+
<>
333+
<Spinner /> Importing...
334+
</>
335+
) : (
336+
"Import"
337+
)}
297338
</Button>
298339
)}
299340
</DialogFooter>
@@ -310,4 +351,23 @@ const ImportComponent = ({
310351
);
311352
};
312353

354+
function createPromiseFromDomEvent(
355+
eventTarget: EventTarget,
356+
eventName: string,
357+
abortSignal?: AbortSignal,
358+
) {
359+
return new Promise<Event>((resolve) => {
360+
const handleEvent = (event: Event) => {
361+
eventTarget.removeEventListener(eventName, handleEvent);
362+
363+
resolve(event);
364+
};
365+
366+
eventTarget.addEventListener(eventName, handleEvent, {
367+
once: true,
368+
signal: abortSignal,
369+
});
370+
});
371+
}
372+
313373
export default ImportComponent;

src/hooks/useComponentUploader.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ interface useComponentUploaderProps {
1717
) => void;
1818
}
1919

20+
/**
21+
* @deprecated - use ComponentLibraryProvider instead
22+
*/
2023
const useComponentUploader = (
2124
readOnly = false,
2225
{ onImportSuccess }: useComponentUploaderProps,

src/hooks/useImportComponent.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ interface ImportComponentProps {
1212
errorCallback?: (error: Error) => void;
1313
}
1414

15+
/**
16+
*
17+
* @deprecated
18+
*/
1519
const useImportComponent = ({
1620
successCallback,
1721
errorCallback,

src/providers/ComponentLibraryProvider/ComponentLibraryProvider.test.tsx

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,48 @@ describe("ComponentLibraryProvider - Component Management", () => {
359359

360360
expect(mockImportComponent).toHaveBeenCalledWith(newComponent);
361361
});
362+
363+
it("should dispatch custom event 'tangle.library.componentAdded' when component is added", async () => {
364+
const newComponent: HydratedComponentReference = {
365+
name: "event-test-component",
366+
digest: "event-digest",
367+
spec: mockComponentSpec,
368+
text: "event test yaml",
369+
};
370+
371+
mockImportComponent.mockResolvedValue(undefined);
372+
mockFlattenFolders.mockReturnValue([]); // No duplicate
373+
374+
const { result } = renderHook(() => useComponentLibrary(), {
375+
wrapper: createWrapper,
376+
});
377+
378+
await waitFor(() => {
379+
expect(result.current.isLoading).toBe(false);
380+
});
381+
382+
const eventListener = vi.fn();
383+
window.addEventListener("tangle.library.componentAdded", eventListener);
384+
385+
await act(async () => {
386+
await result.current.addToComponentLibrary(newComponent);
387+
});
388+
389+
expect(mockImportComponent).toHaveBeenCalledWith(newComponent);
390+
expect(eventListener).toHaveBeenCalled();
391+
392+
// Optional: check event detail contents if relevant
393+
const eventArg = eventListener.mock.calls[0]?.[0];
394+
expect(eventArg).toBeInstanceOf(CustomEvent);
395+
expect(eventArg.detail?.component).toEqual(
396+
expect.objectContaining(newComponent),
397+
);
398+
399+
window.removeEventListener(
400+
"tangle.library.componentAdded",
401+
eventListener,
402+
);
403+
});
362404
});
363405

364406
describe("Removing Components", () => {

src/providers/ComponentLibraryProvider/ComponentLibraryProvider.tsx

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,9 @@ type ComponentLibraryContextType = {
8080
search: string,
8181
filters: string[],
8282
) => SearchResult | null;
83-
addToComponentLibrary: (component: HydratedComponentReference) => void;
83+
addToComponentLibrary: (
84+
component: HydratedComponentReference,
85+
) => Promise<void>;
8486
removeFromComponentLibrary: (component: ComponentReference) => void;
8587
refetchLibrary: () => void;
8688
refetchUserComponents: () => void;
@@ -418,6 +420,25 @@ export const ComponentLibraryProvider = ({
418420
[componentLibrary, userComponentsFolder, usedComponentsFolder],
419421
);
420422

423+
const internalAddComponentToLibrary = useCallback(
424+
async (hydratedComponent: HydratedComponentReference) => {
425+
await importComponent(hydratedComponent);
426+
await refreshComponentLibrary();
427+
await refreshUserComponents();
428+
setNewComponent(null);
429+
setExistingComponent(null);
430+
431+
dispatchEvent(
432+
new CustomEvent("tangle.library.componentAdded", {
433+
detail: {
434+
component: hydratedComponent,
435+
},
436+
}),
437+
);
438+
},
439+
[refreshComponentLibrary, refreshUserComponents, importComponent],
440+
);
441+
421442
const handleImportComponent = useCallback(
422443
async (yamlString: string) => {
423444
try {
@@ -429,11 +450,7 @@ export const ComponentLibraryProvider = ({
429450
throw new Error("Failed to hydrate component");
430451
}
431452

432-
await importComponent(hydratedComponent);
433-
await refreshComponentLibrary();
434-
await refreshUserComponents();
435-
setNewComponent(null);
436-
setExistingComponent(null);
453+
await internalAddComponentToLibrary(hydratedComponent);
437454
} catch (error) {
438455
console.error("Error importing component:", error);
439456
}
@@ -454,19 +471,21 @@ export const ComponentLibraryProvider = ({
454471
)
455472
: undefined;
456473

457-
if (duplicate?.name) {
458-
const existingUserComponent = await getUserComponentByName(
459-
duplicate.name,
460-
);
474+
const existingUserComponent = duplicate?.name
475+
? await getUserComponentByName(duplicate.name)
476+
: undefined;
477+
478+
if (
479+
existingUserComponent &&
480+
existingUserComponent.componentRef.digest !== component.digest
481+
) {
461482
setExistingComponent(existingUserComponent);
462483
setNewComponent(component);
463484
return;
464485
}
465486

466487
try {
467-
await importComponent(component);
468-
await refreshComponentLibrary();
469-
await refreshUserComponents();
488+
await internalAddComponentToLibrary(component);
470489
} catch (error) {
471490
console.error("Error adding component to library:", error);
472491
}
@@ -505,6 +524,8 @@ export const ComponentLibraryProvider = ({
505524
const handleCloseDuplicationDialog = useCallback(() => {
506525
setExistingComponent(null);
507526
setNewComponent(null);
527+
528+
dispatchEvent(new CustomEvent("tangle.library.duplicateDialogClosed"));
508529
}, []);
509530

510531
const searchResult = useMemo(

src/services/componentService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ export const parseComponentData = (data: string): ComponentSpec | null => {
331331
* @param data - The data to normalize.
332332
* @returns The string representation of the data.
333333
*/
334-
function getStringFromData(data: string | ArrayBuffer): string {
334+
export function getStringFromData(data: string | ArrayBuffer): string {
335335
if (typeof data === "object" && "byteLength" in data) {
336336
return new TextDecoder().decode(data);
337337
}

0 commit comments

Comments
 (0)