From acbbf4eb246a8cb3b531037021f4d2530661fb62 Mon Sep 17 00:00:00 2001
From: Yanzhen Yu <yanzhen@smartx.com>
Date: Tue, 19 Jul 2022 19:47:35 +0800
Subject: [PATCH 1/3] fix(runtime): only teardown state when the component is
 not hidden before this check

when we are using slot's ifCondition, the slot may have
unmount traits result multiple times
---
 .../_internal/ImplWrapper/UnmountImplWrapper.tsx          | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/packages/runtime/src/components/_internal/ImplWrapper/UnmountImplWrapper.tsx b/packages/runtime/src/components/_internal/ImplWrapper/UnmountImplWrapper.tsx
index 7603953a..4285c09e 100644
--- a/packages/runtime/src/components/_internal/ImplWrapper/UnmountImplWrapper.tsx
+++ b/packages/runtime/src/components/_internal/ImplWrapper/UnmountImplWrapper.tsx
@@ -33,14 +33,18 @@ export const UnmountImplWrapper = React.forwardRef<HTMLDivElement, ImplWrapperPr
     const traitChangeCallback = useCallback(
       (trait: RuntimeTraitSchema, properties: Record<string, unknown>) => {
         const result = executeTrait(trait, properties);
+        const prevIsHidden = isHidden;
         setIsHidden(!!result.unmount);
         if (result.unmount) {
           // Every component's state is initialized in initStateAnd Method.
           // So if if it should not render, we should remove it from store.
-          delete stateManager.store[c.id];
+          // Only clear the store when the component is not hidden before this check.
+          if (!prevIsHidden) {
+            delete stateManager.store[c.id];
+          }
         }
       },
-      [c.id, executeTrait, stateManager]
+      [c.id, executeTrait, stateManager, isHidden]
     );
 
     useEffect(() => {

From 25787ec1724ce824dfa91f6a20c2947861e676d0 Mon Sep 17 00:00:00 2001
From: Yanzhen Yu <yanzhen@smartx.com>
Date: Tue, 19 Jul 2022 20:11:15 +0800
Subject: [PATCH 2/3] fix(runtime): check whether this is the first time render
 of an UnmountImplWrapper

when we are using slot's ifCondition, the slot may have
unmount traits result multiple times
---
 .../ImplWrapper/UnmountImplWrapper.tsx        | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/packages/runtime/src/components/_internal/ImplWrapper/UnmountImplWrapper.tsx b/packages/runtime/src/components/_internal/ImplWrapper/UnmountImplWrapper.tsx
index 4285c09e..35c0f579 100644
--- a/packages/runtime/src/components/_internal/ImplWrapper/UnmountImplWrapper.tsx
+++ b/packages/runtime/src/components/_internal/ImplWrapper/UnmountImplWrapper.tsx
@@ -1,4 +1,4 @@
-import React, { useCallback, useEffect, useMemo, useState } from 'react';
+import React, { useCallback, useEffect, useMemo, useState, useRef } from 'react';
 import { RuntimeTraitSchema } from '@sunmao-ui/core';
 import { ImplWrapperMain } from './ImplWrapperMain';
 import { useRuntimeFunctions } from './hooks/useRuntimeFunctions';
@@ -11,6 +11,8 @@ export const UnmountImplWrapper = React.forwardRef<HTMLDivElement, ImplWrapperPr
     const { component: c, services, slotContext } = props;
     const { stateManager, registry } = services;
     const { executeTrait } = useRuntimeFunctions(props);
+    const renderCount = useRef(0);
+    renderCount.current++;
 
     const unmountTraits = useMemo(
       () =>
@@ -38,8 +40,19 @@ export const UnmountImplWrapper = React.forwardRef<HTMLDivElement, ImplWrapperPr
         if (result.unmount) {
           // Every component's state is initialized in initStateAnd Method.
           // So if if it should not render, we should remove it from store.
-          // Only clear the store when the component is not hidden before this check.
-          if (!prevIsHidden) {
+
+          /**
+           * prevIsHidden: Only clear the store when the component is not
+           * hidden before this check.
+           *
+           * renderCount: Currently we call initStateAndMethod to setup the
+           * state store, and let here to teardown the hidden components'
+           * state. If a component is hidden forever, it still need to teardown
+           * at the first time it rendered.
+           * Not a perfect solution, and we should have better lifecycle
+           * management for the state store.
+           */
+          if (!prevIsHidden || renderCount.current === 1) {
             delete stateManager.store[c.id];
           }
         }

From 9eb5ab7b46ef808b42419cbd31d2e9f4d16f27ef Mon Sep 17 00:00:00 2001
From: Yanzhen Yu <yanzhen@smartx.com>
Date: Wed, 20 Jul 2022 10:57:13 +0800
Subject: [PATCH 3/3] test(runtime): add unit tests to cover the new teardown
 strategy

when we are using slot's ifCondition, the slot may have
unmount traits result multiple times
---
 .../ImplWrapper/ImplWrapper.spec.tsx          | 46 ++++++++++-
 .../__tests__/ImplWrapper/mockSchema.ts       | 44 ++++++++++
 packages/runtime/__tests__/testLib/Input.tsx  | 13 ++-
 packages/runtime/__tests__/testLib/Tabs.tsx   | 80 +++++++++++++++++++
 packages/runtime/__tests__/testLib/index.ts   |  3 +-
 5 files changed, 182 insertions(+), 4 deletions(-)
 create mode 100644 packages/runtime/__tests__/testLib/Tabs.tsx

diff --git a/packages/runtime/__tests__/ImplWrapper/ImplWrapper.spec.tsx b/packages/runtime/__tests__/ImplWrapper/ImplWrapper.spec.tsx
index e762c6cd..bf38700f 100644
--- a/packages/runtime/__tests__/ImplWrapper/ImplWrapper.spec.tsx
+++ b/packages/runtime/__tests__/ImplWrapper/ImplWrapper.spec.tsx
@@ -1,5 +1,5 @@
 import '@testing-library/jest-dom/extend-expect';
-import { render, screen, waitFor } from '@testing-library/react';
+import { render, screen, waitFor, act } from '@testing-library/react';
 import produce from 'immer';
 import { initSunmaoUI } from '../../src';
 import { TestLib } from '../testLib';
@@ -10,6 +10,7 @@ import {
   HiddenTraitSchema,
   MergeStateSchema,
   AsyncMergeStateSchema,
+  TabsWithSlotsSchema,
 } from './mockSchema';
 
 // A pure single sunmao component will render twice when it mount.
@@ -132,3 +133,46 @@ describe('when component merge state asynchronously', () => {
     clearTesterMap();
   });
 });
+
+describe('slot trait if condition', () => {
+  it('only teardown component state when it is not hidden before the check', () => {
+    const { App, stateManager, apiService } = initSunmaoUI({ libs: [TestLib] });
+    stateManager.noConsoleError = true;
+    const { unmount } = render(<App options={TabsWithSlotsSchema} />);
+    expect(screen.getByTestId('tabs')).toHaveTextContent(`Tab OneTab Two`);
+
+    act(() => {
+      apiService.send('uiMethod', {
+        componentId: 'input',
+        name: 'setValue',
+        parameters: 'new-value',
+      });
+    });
+    expect(stateManager.store).toMatchInlineSnapshot(`
+      Object {
+        "input": Object {
+          "value": "new-value",
+        },
+        "tabs": Object {
+          "selectedTabIndex": 0,
+        },
+      }
+    `);
+    act(() => {
+      screen.getByTestId('tabs-tab-1').click();
+    });
+    expect(stateManager.store).toMatchInlineSnapshot(`
+      Object {
+        "input": Object {
+          "value": "new-value",
+        },
+        "tabs": Object {
+          "selectedTabIndex": 1,
+        },
+      }
+    `);
+
+    unmount();
+    clearTesterMap();
+  });
+});
diff --git a/packages/runtime/__tests__/ImplWrapper/mockSchema.ts b/packages/runtime/__tests__/ImplWrapper/mockSchema.ts
index a2129e29..47ba3644 100644
--- a/packages/runtime/__tests__/ImplWrapper/mockSchema.ts
+++ b/packages/runtime/__tests__/ImplWrapper/mockSchema.ts
@@ -173,3 +173,47 @@ export const AsyncMergeStateSchema: Application = {
     ],
   },
 };
+
+export const TabsWithSlotsSchema: Application = {
+  kind: 'Application',
+  version: 'example/v1',
+  metadata: {
+    name: 'nested_components',
+    description: 'nested components example',
+  },
+  spec: {
+    components: [
+      {
+        id: 'tabs',
+        type: 'test/v1/tabs',
+        properties: {
+          tabNames: ['Tab One', 'Tab Two'],
+          initialSelectedTabIndex: 0,
+        },
+        traits: [],
+      },
+      {
+        id: 'input',
+        type: 'test/v1/input',
+        properties: {
+          text: {
+            raw: 'only in tab {{ $slot.tabIndex + 1 }}',
+            format: 'plain',
+          },
+        },
+        traits: [
+          {
+            type: 'core/v1/slot',
+            properties: {
+              container: {
+                id: 'tabs',
+                slot: 'content',
+              },
+              ifCondition: '{{ $slot.tabIndex === 0 }}',
+            },
+          },
+        ],
+      },
+    ],
+  },
+};
diff --git a/packages/runtime/__tests__/testLib/Input.tsx b/packages/runtime/__tests__/testLib/Input.tsx
index 76ce4d85..4ed57d69 100644
--- a/packages/runtime/__tests__/testLib/Input.tsx
+++ b/packages/runtime/__tests__/testLib/Input.tsx
@@ -23,16 +23,25 @@ export default implementRuntimeComponent({
     state: Type.Object({
       value: Type.String(),
     }),
-    methods: {},
+    methods: {
+      setValue: Type.String(),
+    },
     slots: {},
     styleSlots: [],
     events: [],
   },
-})(({ component, defaultValue, mergeState, elementRef }) => {
+})(({ component, defaultValue, mergeState, elementRef, subscribeMethods }) => {
   const [value, setValue] = useState(defaultValue || '');
   useEffect(() => {
     mergeState({ value });
   }, [mergeState, value]);
+
+  useEffect(() => {
+    subscribeMethods({
+      setValue,
+    });
+  }, [subscribeMethods]);
+
   return (
     <input
       ref={elementRef}
diff --git a/packages/runtime/__tests__/testLib/Tabs.tsx b/packages/runtime/__tests__/testLib/Tabs.tsx
new file mode 100644
index 00000000..bf3a3c0a
--- /dev/null
+++ b/packages/runtime/__tests__/testLib/Tabs.tsx
@@ -0,0 +1,80 @@
+import { implementRuntimeComponent } from '../../src/utils/buildKit';
+import { Type } from '@sinclair/typebox';
+import { useState, useEffect } from 'react';
+
+export default implementRuntimeComponent({
+  version: 'test/v1',
+  metadata: {
+    name: 'tabs',
+    displayName: 'Tabs',
+    description: 'for test',
+    isDraggable: false,
+    isResizable: false,
+    exampleProperties: {},
+    exampleSize: [1, 1],
+    annotations: {
+      category: 'Advance',
+    },
+  },
+  spec: {
+    properties: Type.Object({
+      tabNames: Type.Array(Type.String(), {
+        title: 'Tab Names',
+      }),
+      initialSelectedTabIndex: Type.Number({
+        title: 'Default Selected Tab Index',
+      }),
+    }),
+    state: Type.Object({
+      selectedTabIndex: Type.Number(),
+    }),
+    methods: {},
+    slots: {
+      content: {
+        slotProps: Type.Object({
+          tabIndex: Type.Number(),
+        }),
+      },
+    },
+    styleSlots: [],
+    events: [],
+  },
+})(
+  ({
+    component,
+    tabNames,
+    mergeState,
+    elementRef,
+    initialSelectedTabIndex,
+    slotsElements,
+  }) => {
+    const [selectedTabIndex, setSelectedTabIndex] = useState(
+      initialSelectedTabIndex ?? 0
+    );
+
+    useEffect(() => {
+      mergeState({ selectedTabIndex });
+    }, [mergeState, selectedTabIndex]);
+
+    return (
+      <div ref={elementRef} data-testid={component.id}>
+        <div className="tabs">
+          {tabNames.map((n, idx) => (
+            <div
+              key={n}
+              data-testid={`${component.id}-tab-${idx}`}
+              onClick={() => setSelectedTabIndex(idx)}
+            >
+              {n}
+            </div>
+          ))}
+        </div>
+        <div className="panels">
+          {tabNames.map((n, idx) => (
+            <div key={n}>{slotsElements?.content?.({ tabIndex: idx })}</div>
+          ))}
+        </div>
+      </div>
+    );
+  }
+);
diff --git a/packages/runtime/__tests__/testLib/index.ts b/packages/runtime/__tests__/testLib/index.ts
index a1200fa8..4b4619b1 100644
--- a/packages/runtime/__tests__/testLib/index.ts
+++ b/packages/runtime/__tests__/testLib/index.ts
@@ -2,10 +2,11 @@
 import TestButton from './Button';
 import TestTester from './Tester';
 import TestInput from './Input';
+import TestTabs from './Tabs';
 import TimeoutTrait from './TimeoutTrait';
 import { SunmaoLib } from '../../src';
 
 export const TestLib: SunmaoLib = {
-  components: [TestButton, TestTester, TestInput, TimeoutTrait],
+  components: [TestButton, TestTester, TestInput, TestTabs],
   traits: [TimeoutTrait],
 };