sunmao-ui icon indicating copy to clipboard operation
sunmao-ui copied to clipboard

Proposal: Batch Update For Trait

Open MrWindlike opened this issue 2 years ago • 1 comments

Currently, the same trait would be executed multiple times in a React update cycle. If there are side effects in the trait, some unexpected results may occur. So, this Issue discusses how to solve this problem.

Phenomenon

Let’s take a look the example of this problem. Here are a log trait, it just log the params value when they change.

import { Type } from '@sinclair/typebox';
import { implementRuntimeTrait } from '../../utils/buildKit';
import { CORE_VERSION } from '@sunmao-ui/shared';

export const LogTraitPropertiesSpec = Type.Object({
  param1: Type.Any({
    title: 'Param 1',
  }),
  param2: Type.Any({
    title: 'Param 2',
  }),
});

export default implementRuntimeTrait({
  version: CORE_VERSION,
  metadata: {
    name: 'Log',
    description: '',
    isDataSource: true,
  },
  spec: {
    properties: LogTraitPropertiesSpec,
    state: Type.Any(),
    methods: [],
  },
})(() => {
  return ({ param1, param2 }) => {
    console.log(`log: param1(${param1}), param2(${param2})`);

    return {
      props: {},
    };
  };
});

We pass the {{state.value}} and the {{!state.value}} as the values of the param1 and the param2 . Here is the full App schema:

{
    "kind":"Application",
    "version":"example/v1",
    "metadata":{
        "name":"sunmao application",
        "description":"sunmao empty application"
    },
    "spec":{
        "components":[
            {
                "id":"state",
                "type":"core/v1/dummy",
                "properties":{

                },
                "traits":[
                    {
                        "type":"core/v1/state",
                        "properties":{
                            "key":"value",
                            "initialValue":"value"
                        }
                    }
                ]
            },
            {
                "id":"button",
                "type":"chakra_ui/v1/button",
                "properties":{
                    "text":{
                        "raw":"Button",
                        "format":"plain"
                    },
                    "isLoading":false,
                    "colorScheme":"blue"
                },
                "traits":[
                    {
                        "type":"core/v1/event",
                        "properties":{
                            "handlers":[
                                {
                                    "type":"onClick",
                                    "componentId":"state",
                                    "method":{
                                        "name":"setValue",
                                        "parameters":{
                                            "key":"value",
                                            "value":""
                                        }
                                    },
                                    "wait":{
                                        "type":"debounce",
                                        "time":0
                                    },
                                    "disabled":false
                                }
                            ]
                        }
                    },
                    {
                        "type":"core/v1/Log",
                        "properties":{
                            "param1":"{{state.value}}",
                            "param2":"{{!state.value}}"
                        }
                    }
                ]
            }
        ]
    }
}

After that we click the button to change the state ‘s value once, and then check the logs:

Log.tsx:28 log: param1(), param2(false)
Log.tsx:28 log: param1(), param2(true)

As we can see, it logs twice when the dependent state only changes once. If traits implement the logic of side effects, they can cause unexpected errors, which should not be expected.

Solutions

Batch Execute For The Trait

We should probably wait until all of the Trait's properties have been updated in one React update cycle before executing the Trait. This avoids the problem of executing the Trait multiple times in one React update cycle.

// ImplWrapperMain.tsx
const [traitPropertiesMap, setTraitPropertiesMap] = useState({});

// batch update the properties of traits
useEffect(() => {
  const stops: ReturnType<typeof watch>[] = [];
  const properties: Array<RuntimeTraitSchema['properties']> = [];

  c.traits.forEach((t, i) => {
    const { result, stop } = stateManager.deepEvalAndWatch(
      t.properties,
      ({ result: property }: any) => {
				setTraitPropertiesMap({
					set(traitPropertiesMap, t.type, property);
        })
      },
      {
        slotKey,
        fallbackWhenError: () => undefined,
      }
    );

    stops.push(stop);
    properties.push(result);
  });
	
	setTraitPropertiesMap(c.traits.reduce((result, trait, i)=> {
    result[trait.type] = properties[i];

    return result;
  }, {}));
  
  return () => stops.forEach(s => s());
}, [...]);

// execute traits after the properties changed 
useEffect(()=> {
	c.traits.forEach((t, i) => {
	  const traitResult = executeTrait(t, property);
	  setTraitResults(oldResults => {
	    // assume traits number and order will not change
	    const newResults = [...oldResults];
	    newResults[i] = traitResult;
	    return newResults;
	  });
	});
}, [...]);

But the problem with this solution is that it doesn't execute the Trait until the next iteration of React, which is a potentially risky change from the original mechanism.

Batch Execute For Side Effects

Maybe we don't need to do a batch update on the whole Trait, but only on the side effects. For example, we provide a batch execution method batchExecute for traits to perform side effects.

// trait
const { batchExecute } = props.services;

function sideEffect() {
  ...
}

batchExecute(sideEffect);
// ImplWrapperMain.tsx
const [batchExecuteMap, setBatchExecuteMap] = useState({});

const batchExecute = useCallback((trait, fn) {
  setBatchExecuteQueue((map)=> {
    if (map[trait.type]) return map;

    const newBatchExecuteMap = set(map, trait.type, fn);

    return newBatchExecuteMap;
  });
}, [...]);

useEffect(()=> {
	batchExecuteMap.forEach(fn=> fn());
  setBatchExecuteMap({});
}, [batchExecuteMap]);

Add traitPropertiesDidUpdated

Or we can provide a traitPropertiesDidUpdated lifecycle function to traits to perform side effects.

// ImplWrapperMain.tsx
const [traitPropertiesDidUpdatedHandlers, setTraitPropertiesDidUpdatedHandlers] = useState([]);

// eval traits' properties then execute traits
useEffect(() => {
  const stops: ReturnType<typeof watch>[] = [];
  const properties: Array<RuntimeTraitSchema['properties']> = [];

  c.traits.forEach((t, i) => {
    const { result, stop } = stateManager.deepEvalAndWatch(
      t.properties,
      ({ result: property }: any) => {
        const traitResult = executeTrait(t, property);

        ...
				if (traitResult.traitPropertiesDidUpdated) {
					setTraitPropertiesDidUpdatedHandlers((handlers)=> {
            handlers = handlers.concat(traitResult.traitPropertiesDidUpdated);

						return handlers;
          })
        }
      },
      {
        slotKey,
        fallbackWhenError: () => undefined,
      }
    );
    stops.push(stop);
    properties.push(result);
  });
  ...
}, [...]);

useEffect(() => {
  const clearFunctions = traitPropertiesDidUpdatedHandlers.map(fn => fn());

	setTraitPropertiesDidUpdatedHandlers([]);

  return () => {
    clearFunctions?.forEach(func => func && func());
  };
}, [traitPropertiesDidUpdatedHandlers]);

Don’t Change The Code In Runtime

The final solution is that we don't deal with this issue at runtime, but instead ask users to write traits without executing logic that has side effects. If the users need to perform side effects, they can write components to do so.

Conclusion

I prefer the third option, which is to provide a new lifecycle function traitPropertiesDidUpdated to traits.

The reasons are as follows:

The first solution solves the problem, but it changes the original implementation mechanism and has the possibility of causing the original project to go wrong unexpectedly.

The second option does not break the original mechanism, but providing a single function to handle side effects may increase the user's understanding cost.

The third, more intuitive solution is to provide a new lifecycle function that executes when a Trait property changes, allowing users to do anything in it, including side effects that they don't want to repeat.

The last one requires users to modify their own code and is unfriendly because it doesn't address the limitations of traits.

MrWindlike avatar Jan 03 '23 01:01 MrWindlike