Skip to content

Commit

Permalink
fix(state): Don't invoke user event handler during state initialization
Browse files Browse the repository at this point in the history
Issue: https://linear.app/plasmic/issue/PLA-11395
GitOrigin-RevId: 4a82add2fe6c407c591230511c60cd1b44aeb296
  • Loading branch information
FMota0 authored and actions-user committed Dec 16, 2024
1 parent 4411fd2 commit 03fbef8
Show file tree
Hide file tree
Showing 5 changed files with 140 additions and 57 deletions.
1 change: 1 addition & 0 deletions packages/react-web/src/states/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,5 @@ export interface Internal$State {
stack: string[];
visited: Set<string>;
};
initializedLeafPaths: Set<string>;
}
13 changes: 12 additions & 1 deletion packages/react-web/src/states/valtio.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,17 @@ function create$StateProxy(
).set?.(target, property, value, receiver);
Reflect.set(target, property, value, receiver);
if (nextSpec?.onChangeProp) {
$$state.env.$props[nextSpec.onChangeProp]?.(value);
const pathKey = JSON.stringify(nextPath);
const isInitOnChange = !$$state.initializedLeafPaths.has(pathKey);

// We need to call the onChangeProp during initialization process so that the parent
// state can be updated with the correct value. We will provide an addtionnal parameter
// to the onChangeProp function to indicate that the call is made during initialization.
$$state.env.$props[nextSpec.onChangeProp]?.(value, isInitOnChange);

if (isInitOnChange) {
$$state.initializedLeafPaths.add(pathKey);
}
}
}
if (!nextNode) {
Expand Down Expand Up @@ -460,6 +470,7 @@ export function useDollarState(
specs: [],
registrationsQueue: [],
stateInitializationEnv: { stack: [], visited: new Set<string>() },
initializedLeafPaths: new Set(),
};
})()
).current;
Expand Down
56 changes: 42 additions & 14 deletions platform/wab/src/wab/client/components/canvas/canvas-rendering.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,11 @@ import {
makeWabTextClassName,
} from "@/wab/shared/codegen/react-p/serialize-utils";
import { deriveReactHookSpecs } from "@/wab/shared/codegen/react-p/utils";
import { paramToVarName, toVarName } from "@/wab/shared/codegen/util";
import {
paramToVarName,
toJsIdentifier,
toVarName,
} from "@/wab/shared/codegen/util";
import {
assert,
cx,
Expand Down Expand Up @@ -157,6 +161,7 @@ import { makeSelectableKey } from "@/wab/shared/core/selection";
import { isSlotSelection } from "@/wab/shared/core/slots";
import {
StateVariableType,
getComponentStateOnChangePropNames,
getLastPartOfImplicitStateName,
getStateDisplayName,
getStateOnChangePropName,
Expand Down Expand Up @@ -268,6 +273,7 @@ import {
import { hashExpr } from "@/wab/shared/site-diffs";
import { PageSizeType, deriveSizeStyleValue } from "@/wab/shared/sizingutils";
import { placeholderImgUrl } from "@/wab/shared/urls";
import { JsIdentifier } from "@/wab/shared/utils/regex-js-identifier";
import {
makeVariantComboSorter,
sortedVariantSettings,
Expand Down Expand Up @@ -1617,7 +1623,13 @@ function renderTplComponent(
ctx.sub.reactWeb.generateStateValueProp(ctx.env.$state, statePath);
}
});
mergeEventHandlers(props, builtinEventHandlers);
if (ctx.ownerComponent) {
mergeEventHandlers(
props,
builtinEventHandlers,
getComponentStateOnChangePropNames(ctx.ownerComponent, node)
);
}

ctx.ownerComponent?.states.forEach((state) => {
if (state.tplNode !== node) {
Expand Down Expand Up @@ -2228,22 +2240,38 @@ function canvasParamToVarName(

function mergeEventHandlers(
userAttrs: Record<string, any>,
builtinEventHandlers: Record<string, any[]>
builtinEventHandlers: Record<string, any[]>,
onChangeAttrs: Set<JsIdentifier> = new Set()
) {
const chained = (handlers: any[]) => {
if (handlers.length === 1) {
return handlers[0];
} else {
return async (...args: unknown[]) => {
for (const handler of handlers) {
await handler.apply(null, args);
}
};
}
const chained = (
attr: string,
attrBuiltinEventHandlers: any[],
userAttr: any[]
) => {
return async (...args: unknown[]) => {
for (const handler of attrBuiltinEventHandlers) {
await handler.apply(null, args);
}

// Check if we should skip user attr handlers because of state initialization trigger
if (
onChangeAttrs.has(toJsIdentifier(attr)) &&
args.length > 1 &&
args[1]
) {
return;
}

for (const handler of userAttr) {
await handler.apply(null, args);
}
};
};
Object.keys(builtinEventHandlers).forEach((key) => {
userAttrs[key] = chained(
withoutNils([...builtinEventHandlers[key], userAttrs[key]])
toJsIdentifier(key),
withoutNils(builtinEventHandlers[key]),
withoutNils([userAttrs[key]])
);
});
}
Expand Down
51 changes: 39 additions & 12 deletions platform/wab/src/wab/shared/codegen/react-p/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ import {
allStyleTokens,
} from "@/wab/shared/core/sites";
import {
getComponentStateOnChangePropNames,
getLastPartOfImplicitStateName,
getStateDisplayName,
getStateOnChangePropName,
Expand Down Expand Up @@ -314,6 +315,7 @@ import {
deriveSizeStyleValue,
getPageComponentSizeType,
} from "@/wab/shared/sizingutils";
import { JsIdentifier } from "@/wab/shared/utils/regex-js-identifier";
import { makeVariantComboSorter } from "@/wab/shared/variant-sort";
import L from "lodash";
import { shouldUsePlasmicImg } from "src/wab/shared/codegen/react-p/image";
Expand Down Expand Up @@ -1470,23 +1472,43 @@ export function serializeTplTagBase(

function mergeEventHandlers(
userAttrs: Record<string, string>,
builtinEventHandlers: Record<string, string[]>
builtinEventHandlers: Record<string, string[]>,
onChangeAttrs: Set<JsIdentifier> = new Set()
) {
const chained = (handlerCodes: string[]) => {
if (handlerCodes.length === 1) {
return handlerCodes[0];
} else {
return `async (...eventArgs: any) => {
${handlerCodes
.map((code) => `(${code}).apply(null, eventArgs);`)
.join("\n")}
}`;
const chainedFunctionCall = (code: string) =>
`(${code}).apply(null, eventArgs);`;

// When dealing with event handlers for plasmic state updates, we won't call the user's event handler
// during the state initialization phase. This is provided by the second argument of the event handler
// which is a boolean indicating whether the onChange event is triggered during the state initialization phase.
const maybeHaltUserAttr = (attr: string) => {
if (onChangeAttrs.has(toJsIdentifier(attr))) {
return `if(eventArgs.length > 1 && eventArgs[1]) {
return;
}`;
}
return "";
};

const chained = (
attr: string,
attrBuiltinEventHandlers: string[],
userAttr: string[]
) => {
return `async (...eventArgs: any) => {
${attrBuiltinEventHandlers.map(chainedFunctionCall).join("\n")}
${maybeHaltUserAttr(attr)}
${userAttr.map(chainedFunctionCall).join("\n")}
}`;
};

for (const key of Object.keys(builtinEventHandlers)) {
userAttrs[key] = chained(
withoutNils([...builtinEventHandlers[key], userAttrs[key]])
key,
withoutNils(builtinEventHandlers[key]),
withoutNils([userAttrs[key]])
);
}
}
Expand Down Expand Up @@ -1907,7 +1929,12 @@ export function serializeTplComponentBase(
}`;
}
});
mergeEventHandlers(attrs, builtinEventHandlers);

mergeEventHandlers(
attrs,
builtinEventHandlers,
getComponentStateOnChangePropNames(ctx.component, node)
);
}

if (
Expand Down
76 changes: 46 additions & 30 deletions platform/wab/src/wab/shared/core/states.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
import { TplMgr } from "@/wab/shared/TplMgr";
import { $$$ } from "@/wab/shared/TplQuery";
import {
isStandaloneVariantGroup,
tryGetBaseVariantSetting,
} from "@/wab/shared/Variants";
import { AddItemKey } from "@/wab/shared/add-item-keys";
import { toVarName } from "@/wab/shared/codegen/util";
import {
assert,
assertNever,
Expand All @@ -6,18 +14,18 @@ import {
remove,
unexpected,
uniqueName,
withoutNils,
} from "@/wab/shared/common";
import {
getComponentDisplayName,
removeComponentParam,
} from "@/wab/shared/core/components";
import { DevFlagsType } from "@/wab/shared/devflags";
import {
ExprCtx,
InteractionConditionalMode,
asCode,
code,
codeLit,
ExprCtx,
InteractionConditionalMode,
isFallbackSet,
isRealCodeExpr,
isRealCodeExprEnsuringType,
Expand All @@ -26,33 +34,19 @@ import {
} from "@/wab/shared/core/exprs";
import * as Lang from "@/wab/shared/core/lang";
import { ParamExportType } from "@/wab/shared/core/lang";
import { AddItemKey } from "@/wab/shared/add-item-keys";
import { toVarName } from "@/wab/shared/codegen/util";
import { extractComponentUsages, writeable } from "@/wab/shared/core/sites";
import * as Tpls from "@/wab/shared/core/tpls";
import { isTplComponent } from "@/wab/shared/core/tpls";
import { DevFlagsType } from "@/wab/shared/devflags";
import { parseExpr } from "@/wab/shared/eval/expression-parser";
import { ensureComponentsObserved } from "@/wab/shared/mobx-util";
import {
Component,
ComponentVariantGroup,
CustomCode,
ensureKnownFunctionType,
ensureKnownNamedState,
EventHandler,
Expr,
Interaction,
isKnownCollectionExpr,
isKnownCustomCode,
isKnownFunctionArg,
isKnownFunctionExpr,
isKnownGenericEventHandler,
isKnownImageAssetRef,
isKnownNamedState,
isKnownObjectPath,
isKnownPageHref,
isKnownTplComponent,
isKnownTplRef,
isKnownTplTag,
isKnownVariantsRef,
isKnownVarRef,
NameArg,
NamedState,
Param,
Expand All @@ -66,17 +60,24 @@ import {
TplTag,
VariantGroup,
VariantGroupState,
ensureKnownFunctionType,
ensureKnownNamedState,
isKnownCollectionExpr,
isKnownCustomCode,
isKnownFunctionArg,
isKnownFunctionExpr,
isKnownGenericEventHandler,
isKnownImageAssetRef,
isKnownNamedState,
isKnownObjectPath,
isKnownPageHref,
isKnownTplComponent,
isKnownTplRef,
isKnownTplTag,
isKnownVarRef,
isKnownVariantsRef,
} from "@/wab/shared/model/classes";
import { TplMgr } from "@/wab/shared/TplMgr";
import { $$$ } from "@/wab/shared/TplQuery";
import {
isStandaloneVariantGroup,
tryGetBaseVariantSetting,
} from "@/wab/shared/Variants";
import { extractComponentUsages, writeable } from "@/wab/shared/core/sites";
import { smartHumanize } from "@/wab/shared/strs";
import * as Tpls from "@/wab/shared/core/tpls";
import { isTplComponent } from "@/wab/shared/core/tpls";
import { getPublicUrl } from "@/wab/shared/urls";
import { isArray } from "lodash";

Expand Down Expand Up @@ -498,6 +499,21 @@ export function getStateOnChangePropName(state: State) {
return onChangeParam ? toVarName(onChangeParam.variable.name) : undefined;
}

export function getComponentStateOnChangePropNames(
component: Component,
node: TplComponent
) {
return new Set(
withoutNils(
component.states.map((state) =>
state.tplNode === node && !!state.implicitState
? getStateOnChangePropName(state.implicitState)
: null
)
)
);
}

export function isPrivateState(state: State) {
return state.accessType === "private";
}
Expand Down

0 comments on commit 03fbef8

Please sign in to comment.