fix(joint-react): features mount problem

packages/joint-react/src/hooks/__tests__/use-create-features.test.tsx

@@ -178,7 +178,7 @@ describe('useCreateFeature (target: paper)', () => {
     });
   });
 
-  it('calls onUpdateFeature when dependencies change', async () => {
+  it('calls onUpdateFeature when dependencies change (not on initial mount)', async () => {
     const onUpdateFeature = jest.fn();
     const mockInstance = { count: 0 };
 
@@ -201,15 +201,16 @@ describe('useCreateFeature (target: paper)', () => {
     const Wrapper = createPaperWrapper();
     const { rerender } = render(<TestComponent dep={1} />, { wrapper: Wrapper });
 
+    // onUpdateFeature should NOT be called on initial mount
     await waitFor(() => {
-      expect(onUpdateFeature).toHaveBeenCalled();
+      expect(onUpdateFeature).not.toHaveBeenCalled();
     });
 
-    const callsAfterMount = onUpdateFeature.mock.calls.length;
+    // Trigger a dependency change
     rerender(<TestComponent dep={2} />);
 
     await waitFor(() => {
-      expect(onUpdateFeature.mock.calls.length).toBeGreaterThan(callsAfterMount);
+      expect(onUpdateFeature).toHaveBeenCalledTimes(1);
       expect(onUpdateFeature).toHaveBeenLastCalledWith(
         expect.objectContaining({
           graphStore: expect.any(Object),
@@ -225,6 +226,59 @@ describe('useCreateFeature (target: paper)', () => {
 // Graph target
 // ────────────────────────────────────────────────────────────────────────────
 
+// ────────────────────────────────────────────────────────────────────────────
+// onAddFeature call count
+// ────────────────────────────────────────────────────────────────────────────
+
+describe('onAddFeature is called exactly once', () => {
+  it('paper target: onAddFeature fires once on mount', async () => {
+    const onAddFeature = jest.fn(() => ({
+      id: 'once-paper',
+      instance: { value: 1 },
+    }));
+
+    function TestComponent() {
+      useCreateFeature('paper', { id: 'once-paper', onAddFeature });
+      return null;
+    }
+
+    const Wrapper = createPaperWrapper();
+    render(<TestComponent />, { wrapper: Wrapper });
+
+    // Wait for feature to be registered
+    await waitFor(() => {
+      expect(onAddFeature).toHaveBeenCalled();
+    });
+
+    expect(onAddFeature).toHaveBeenCalledTimes(1);
+  });
+
+  it('graph target: onAddFeature fires once on mount', async () => {
+    const onAddFeature = jest.fn(() => ({
+      id: 'once-graph',
+      instance: { value: 1 },
+    }));
+
+    function TestComponent() {
+      useCreateFeature('graph', { id: 'once-graph', onAddFeature });
+      return null;
+    }
+
+    const Wrapper = createGraphWrapper();
+    render(<TestComponent />, { wrapper: Wrapper });
+
+    await waitFor(() => {
+      expect(onAddFeature).toHaveBeenCalled();
+    });
+
+    expect(onAddFeature).toHaveBeenCalledTimes(1);
+  });
+});
+
+// ────────────────────────────────────────────────────────────────────────────
+// Graph target
+// ────────────────────────────────────────────────────────────────────────────
+
 describe('useCreateFeature (target: graph)', () => {
   it('registers a feature with the graph store', async () => {
     const mockInstance = { doSomething: jest.fn() };
@@ -332,7 +386,7 @@ describe('useCreateFeature (target: graph)', () => {
     });
   });
 
-  it('calls onUpdateFeature when dependencies change', async () => {
+  it('calls onUpdateFeature when dependencies change (not on initial mount)', async () => {
     const onUpdateFeature = jest.fn();
     const mockInstance = { count: 0 };
 
@@ -355,15 +409,16 @@ describe('useCreateFeature (target: graph)', () => {
     const Wrapper = createGraphWrapper();
     const { rerender } = render(<TestComponent dep={1} />, { wrapper: Wrapper });
 
+    // onUpdateFeature should NOT be called on initial mount
     await waitFor(() => {
-      expect(onUpdateFeature).toHaveBeenCalled();
+      expect(onUpdateFeature).not.toHaveBeenCalled();
     });
 
-    const callsAfterMount = onUpdateFeature.mock.calls.length;
+    // Trigger a dependency change
     rerender(<TestComponent dep={2} />);
 
     await waitFor(() => {
-      expect(onUpdateFeature.mock.calls.length).toBeGreaterThan(callsAfterMount);
+      expect(onUpdateFeature).toHaveBeenCalledTimes(1);
       expect(onUpdateFeature).toHaveBeenLastCalledWith(
         expect.objectContaining({
           graphStore: expect.any(Object),

packages/joint-react/src/hooks/use-create-features.ts

@@ -235,13 +235,30 @@ export function useCreateFeature<T>(
 
   const asChildren = !!paperStore;
 
-  // Create and register the feature
+  // Guard: skip onUpdateFeature on initial mount — it must only fire on dependency changes
+  const isMountedRef = useRef(false);
+  // Holds the created feature to survive strict-mode cleanup/re-mount without re-calling onAddFeature
+  const featureRef = useRef<Feature | null>(null);
+
+  // Create and register the feature (fires onAddFeature exactly once)
   useLayoutEffect(() => {
     if (isPaperTarget && !paperStore) return;
+
+    // Re-register cached feature if it was removed by strict-mode cleanup
+    if (featureRef.current) {
+      registerFeature(target, graphStore, paperStore, featureRef.current);
+      setForwardRef(forwardedRef, featureRef.current.instance);
+      return () => unregisterFeature(target, graphStore, paperStore, featureRef.current!.id);
+    }
+
     const feature = createAndRegisterFeature(target, onAddFeature, graphStore, paperStore, asChildren);
+    featureRef.current = feature;
     registerFeature(target, graphStore, paperStore, feature);
     setForwardRef(forwardedRef, feature.instance);
-    return () => unregisterFeature(target, graphStore, paperStore, feature.id);
+    return () => {
+      isMountedRef.current = false;
+      unregisterFeature(target, graphStore, paperStore, feature.id);
+    };
   }, [graphStore, paperStore]);
 
   // Fire onLoad when feature instance becomes available
@@ -252,16 +269,23 @@ export function useCreateFeature<T>(
     fireOnLoad(target, onLoad, graphStore, paperStore, resolvedFeature, asChildren);
   }, [resolvedFeature]);
 
-  // Fire onUpdateFeature when dependencies change
+  // Fire onUpdateFeature ONLY when dependencies change after mount.
+  // Never fires on initial mount — onAddFeature handles creation.
+  // resolvedFeature is intentionally NOT in deps to prevent spurious fires
+  // when the feature instance first resolves (null → instance).
   useLayoutEffect(() => {
     if (!onUpdateFeature) return;
     if (isPaperTarget && !paperStore) return;
+    if (!isMountedRef.current) {
+      isMountedRef.current = true;
+      return;
+    }
     const existingFeature = resolveExistingFeature(target, graphStore, paperStore, id);
     fireOnUpdate(target, onUpdateFeature, graphStore, paperStore, existingFeature?.instance, asChildren);
     if (existingFeature) {
       registerFeature(target, graphStore, paperStore, existingFeature);
     }
-  }, [graphStore, resolvedFeature, paperStore, ...dependencies]);
+  }, [graphStore, paperStore, ...dependencies]);
 
   return featureContext;
 }