Skip to content

Commit

Permalink
Added a workaround for the potential bug in Show causing double evalu…
Browse files Browse the repository at this point in the history
…ation of the condition: solidjs/solid#2406
  • Loading branch information
TPReal committed Jan 20, 2025
1 parent 7934299 commit 46b8523
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 26 deletions.
2 changes: 1 addition & 1 deletion resources/js/components/ui/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const EditButton: VoidComponent<EditButtonProps> = (allProps) => {
const t = useLangFunc();
return (
<Button {...buttonProps}>
<actionIcons.Edit class="inlineIcon" /> {props.label === undefined ? t("actions.edit") : props.label}
<actionIcons.Edit class="inlineIcon" /> {props.label ?? t("actions.edit")}
</Button>
);
};
Expand Down
29 changes: 16 additions & 13 deletions resources/js/components/ui/Modal.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {cx, useLangFunc} from "components/utils";
import {Show_noDoubleEvaluation} from "components/utils/workarounds";
import {VsClose} from "solid-icons/vs";
import {Accessor, createComputed, createMemo, createSignal, createUniqueId, JSX, onCleanup, Show} from "solid-js";
import {Portal} from "solid-js/web";
Expand Down Expand Up @@ -252,19 +253,21 @@ export const Modal = <T, C extends CloseReason>(props: Props<T, C>): JSX.Element
"top": `${relativePos()[1]}px`,
}}
>
<Show when={props.title}>
<h2
class={cx(
"font-bold select-none touch-none pr-4",
canDrag() ? "cursor-grab" : undefined,
closeOn().has("closeButton") ? "pr-8" : undefined,
)}
style={{"font-size": "1.3rem"}}
{...grabHandler}
>
{props.title}
</h2>
</Show>
<Show_noDoubleEvaluation when={props.title}>
{(title) => (
<h2
class={cx(
"font-bold select-none touch-none pr-4",
canDrag() ? "cursor-grab" : undefined,
closeOn().has("closeButton") ? "pr-8" : undefined,
)}
style={{"font-size": "1.3rem"}}
{...grabHandler}
>
{title()}
</h2>
)}
</Show_noDoubleEvaluation>
<div
// Grab handler covering the top margin of the dialog.
class={cx("absolute top-0 right-4 left-4 h-4 touch-none", canDrag() ? "cursor-grab" : undefined)}
Expand Down
9 changes: 7 additions & 2 deletions resources/js/components/ui/Table/Table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {createScrollableUpMarker} from "components/ui/ScrollableUpMarker";
import {currentTimeSecond, cx, delayedAccessor, useLangFunc} from "components/utils";
import {featureUseTrackers} from "components/utils/feature_use_trackers";
import {NonBlocking} from "components/utils/NonBlocking";
import {Show_noDoubleEvaluation} from "components/utils/workarounds";
import {TOptions} from "i18next";
import {
Accessor,
Expand Down Expand Up @@ -279,7 +280,9 @@ export const Table = <T,>(allProps: VoidProps<Props<T>>): JSX.Element => {
<TableContext.Provider value={props.table}>
<Show when={!props.isLoading} fallback={<BigSpinner />}>
<div class={cx(s.tableContainer, s[props.mode])}>
<Show when={props.aboveTable?.()}>{(aboveTable) => <div class={s.aboveTable}>{aboveTable()}</div>}</Show>
<Show_noDoubleEvaluation when={props.aboveTable?.()}>
{(aboveTable) => <div class={s.aboveTable}>{aboveTable()}</div>}
</Show_noDoubleEvaluation>
<div class={s.tableMain}>
<div
ref={(div) => {
Expand Down Expand Up @@ -361,7 +364,9 @@ export const Table = <T,>(allProps: VoidProps<Props<T>>): JSX.Element => {
<LoadingPane isLoading={props.isDimmed} />
</div>
</div>
<Show when={props.belowTable?.()}>{(belowTable) => <div class={s.belowTable}>{belowTable()}</div>}</Show>
<Show_noDoubleEvaluation when={props.belowTable?.()}>
{(belowTable) => <div class={s.belowTable}>{belowTable()}</div>}
</Show_noDoubleEvaluation>
</div>
</Show>
</TableContext.Provider>
Expand Down
16 changes: 6 additions & 10 deletions resources/js/components/utils/Recreator.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,14 @@
import {ChildrenOrFunc, getChildrenElement} from "components/ui/children_func";
import {createMemo, on} from "solid-js";
import {Show} from "solid-js";

interface Props<T> {
readonly signal: T;
readonly children: ChildrenOrFunc<[T]>;
}

/** Recreates the children every time the signal is changed. */
export const Recreator = <T,>(props: Props<T>) => {
const content = createMemo(
on(
() => props.signal,
(signal) => getChildrenElement(props.children, signal),
),
);
return <>{content()}</>;
};
export const Recreator = <T,>(props: Props<T>) => (
<Show keyed when={[props.signal] as const}>
{([signal]) => getChildrenElement(props.children, signal)}
</Show>
);
20 changes: 20 additions & 0 deletions resources/js/components/utils/workarounds.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import {ChildrenOrFunc} from "components/ui/children_func";
import {Accessor, createMemo, JSX, Show} from "solid-js";

interface Show_noDoubleEvaluationProps<T> {
readonly when: T;
readonly children: ChildrenOrFunc<[Accessor<NonNullable<T>>]>;
readonly fallback?: JSX.Element;
}

/**
* A version of <Show> that is not affected by the (potential) bug where the condition is evaluated
* twice at start if the function form is used. See https://github.com/solidjs/solid/issues/2406
*
* It makes sense to use this replacement if the condition is expensive to evaluate, or if
* the condition creates JSX (e.g. it is of type JSX.Element) to avoid creating two copies of the JSX.
*/
export const Show_noDoubleEvaluation = <T,>(props: Show_noDoubleEvaluationProps<T>) => {
const when = createMemo(() => props.when);
return <Show {...props} when={when()} />;
};

0 comments on commit 46b8523

Please sign in to comment.