1014-memoize-conditional-branch-widgets #1018
Conversation
🦋 Changeset detectedLatest commit: 078c704 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Visit the preview URL for this PR (updated for commit 078c704): https://react-kitchen-sink-dev--pr1018-1014-memoize-conditi-fr68kirn.web.app (expires Thu, 13 Feb 2025 06:23:36 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 6267897ade2ba783b6db70a53a60fc3946d625e9 |
| }, [conditionStatements, conditions, trueIndex]); | ||
|
|
||
| if (!widget) { | ||
| return null; | ||
| } | ||
|
|
||
| return <>{EnsembleRuntime.render([widget])}</>; | ||
| return <>{widget}</>; |
There was a problem hiding this comment.
Don't need a fragment here anymore.
There was a problem hiding this comment.
Then we also need to update the return type of the Conditional widget from React.FC. is this fine ??
There was a problem hiding this comment.
Why would that be the case? widget should be a ReactNode
There was a problem hiding this comment.
The EnsembleRuntime.render function returns ReactNode[]. When we save this in our widget memo, it will contain either that array of nodes or null. so we need to wrap the output in a fragment <>...</>.
There was a problem hiding this comment.
I'm not sure why this matters since ReactNode[] is also a valid ReactNode.
type ReactNode =
| ReactElement
| string
| number
| Iterable
| ReactPortal
| boolean
| null
| undefined
| DO_NOT_USE_OR_YOU_WILL_BE_FIRED_EXPERIMENTAL_REACT_NODES[
keyof DO_NOT_USE_OR_YOU_WILL_BE_FIRED_EXPERIMENTAL_REACT_NODES
];
…sembleUI/ensemble-react into 1014-memoize-conditional-branch-widgets
|
|
||
| fireEvent.click(screen.getByText("Decrease")); | ||
| expect(unwrapWidgetSpy).toHaveBeenCalledTimes(3); | ||
| expect(screen.getByText("Less than 0")).not.toBeNull(); |
There was a problem hiding this comment.
Can you add assertion about how many times EnsembleRuntime.render was called?
Describe your changes
Memoize conditional branch widgets
Screenshots [Optional]
Issue ticket number and link
Closes #1014
Checklist before requesting a review
pnpm changeset add