Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf[react-native]: Memoized Blocks Component to free up UI thread. #3814

Merged
merged 7 commits into from
Jan 24, 2025
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix: merge conflicts
Yash Wadhia authored and Yash Wadhia committed Jan 21, 2025
commit 81a7a82fddfea1680ba268660fe015caa363a478
71 changes: 71 additions & 0 deletions packages/sdks/mitosis.config.cjs
Original file line number Diff line number Diff line change
@@ -163,6 +163,76 @@ const ADD_IS_STRICT_STYLE_MODE_TO_CONTEXT_PLUGIN = () => ({
},
});

const MEMOIZING_BLOCKS_COMPONENT_PLUGIN = () => ({
json: {
post: (json) => {
if (json.name === 'Blocks') {
json.imports.push({
imports: { memo: 'memo' },
path: 'react',
});

json.hooks.init = {
code: `
${json.hooks.init?.code || ''}
const renderItem = React.useCallback(({ item }: { item: any }) => (
<Block
block={item}
linkComponent={props.linkComponent}
context={props.context || builderContext}
registeredComponents={
props.registeredComponents || componentsContext?.registeredComponents
}
/>
), [
props.linkComponent,
props.context,
props.registeredComponents,
builderContext,
componentsContext?.registeredComponents
]);

// Memoize keyExtractor
const keyExtractor = React.useCallback((item: any) =>
item.id.toString()
, []);
`,
};

json.children[0].children[0].children[0] = {
'@type': '@builder.io/mitosis/node',
name: 'FlatList',
meta: {},
scope: {},
properties: {},
bindings: {
data: { code: 'props.blocks', type: 'single' },
renderItem: { code: 'renderItem', type: 'single' },
keyExtractor: { code: 'keyExtractor', type: 'single' },
removeClippedSubviews: { code: 'true', type: 'single' },
maxToRenderPerBatch: { code: '10', type: 'single' },
windowSize: { code: '5', type: 'single' },
initialNumToRender: { code: '5', type: 'single' }
},
children: []
}
}
return json;
},
},
code: {
post: (code, json) => {
if (json.name === 'Blocks') {
return code.replace(
'export default Blocks',
'export default memo(Blocks)'
);
}
return code;
},
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially in this PR, you were memoizing Block, but now you're memoizing Blocks instead.

Was this intended? Do we still get the same performance benefits from memoizing the parent component instead of the individual items?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Sami, In the earlier PR also we were memoizing Blocks.tsx and not Block.tsx

Copy link
Contributor

@samijaber samijaber Jan 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah indeed, you memoized Blocks.tsx in your original PR commit.

but you were also creating a memoized version of Block called RenderBlock : https://github.com/BuilderIO/builder/pull/3814/commits/e264c120fd38851208875a9915b55e26e952528f#diff-b9cbe94aa51d49deda0[…]fd13d38a1f56e5641441f5R20

CleanShot 2025-01-21 at 07 25 26@2x

Which is now gone from this PR. Was it unnecessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah my bad I missed this part. Also one question I want to ask by using mitosis how can I write code outside the Blocks function but inside the Blocks.tsx

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in my Loom I explan that in this case, you probably want to add export default Memo(Block) to Block.tsx, and remove this RenderBlock from Blocks.tsx to achieve the same outcome

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha!
Asking out of curiosity. Is there a way where we can declare variables/functions outside Functional/Class component using mitosis.config.js?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes actually, there's a preComponent hook in Mitosis whose code will be printed right after import statements (and therefore, outside React components)

});

const INJECT_ENABLE_EDITOR_ON_EVENT_HOOKS_PLUGIN = () => ({
json: {
pre: (json) => {
@@ -892,6 +962,7 @@ module.exports = {
INJECT_ENABLE_EDITOR_ON_EVENT_HOOKS_PLUGIN,
REMOVE_SET_CONTEXT_PLUGIN_FOR_FORM,
ADD_IS_STRICT_STYLE_MODE_TO_CONTEXT_PLUGIN,
MEMOIZING_BLOCKS_COMPONENT_PLUGIN,
() => ({
json: {
pre: (json) => {
2 changes: 1 addition & 1 deletion packages/sdks/output/react-native/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@builder.io/sdk-react-native",
"description": "Builder.io SDK for React Native",
"version": "3.0.3",
"version": "3.0.1-1",
"homepage": "https://github.com/BuilderIO/builder/tree/main/packages/sdks/output/react-native",
"repository": {
"type": "git",

This file was deleted.

Loading