-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: replace duplicate componentInfos with registeredComponents #4167
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 47bee42 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 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 |
fab4d33 to
2390a52
Compare
2390a52 to
47bee42
Compare
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx test @e2e/nextjs-sdk-next-app |
❌ Failed | 23m 9s | View ↗ |
nx test @snippet/nextjs-sdk-next-app |
❌ Failed | 15m 47s | View ↗ |
nx test @snippet/angular-17 |
❌ Failed | 5m 44s | View ↗ |
nx test @snippet/angular-17-ssr |
❌ Failed | 5m 32s | View ↗ |
nx test @snippet/vue |
❌ Failed | 3m 33s | View ↗ |
nx test @snippet/svelte |
❌ Failed | 2m 26s | View ↗ |
nx test @snippet/react |
❌ Failed | 1m 58s | View ↗ |
nx test @e2e/qwik-city |
✅ Succeeded | 9m 25s | View ↗ |
Additional runs (37) |
✅ Succeeded | ... | View ↗ |
☁️ Nx Cloud last updated this comment at 2025-10-30 10:13:52 UTC
samijaber
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @intellix.
This PR is failing CI tests for the NextJS SDK
The reason we create componentInfos is to split the Component (actual React component) from its Builder metadata (name, inputs, styles, etc.)
If you look closely, you'll see that componentInfos includes everything except the .component field, whereas state.registeredComponents includes component + metadata.
Therefore, the componentInfos state is safe to pass between RSC (React Server Components) and Client components, whereas registeredComponents might include RSCs and can therefore not be passed down as a prop to client components.
To your point, I do see that state.registeredComponents includes the metadata unnecessarily. In theory, we should be able to omit the metadata from there, and make the necessary updates throughout the SDK so that any usage of that metadata will rely on props.componentInfos instead.
That should reduce the amount of data stored (hopefully by as much as your current PR does), while still allowing the NextJS SDK to work as expected.
If you want to attempt such a change, the best way to make sure you address all the required changes is:
- running
yarn typecheck:watchinpackages/sdks - modifying
state.registeredComponents:
//from
registeredComponents: [
...getDefaultRegisteredComponents(),
...(props.customComponents || []),
].reduce<RegisteredComponents>(
(acc, { component, ...info }) => ({
...acc,
[info.name]: {
component: useTarget({
vue: wrapComponentRef(component),
default: component,
}),
...serializeIncludingFunctions(info),
},
}),
{}
)
// to
registeredComponents: [
...getDefaultRegisteredComponents(),
...(props.customComponents || []),
].reduce<Dictionary<{ component: ComponentReference }>>(
(acc, { component, ...info }) => ({
...acc,
[info.name]: {
component: useTarget({
vue: wrapComponentRef(component),
default: component,
}),
},
}),
{}
)and making all the necessary followup changes throughout the SDK.
|
Thanks for reading through it and detailing the changes required :) We've got a big project to deliver and already hacked this away in a PNPM patch (linked in my Issue) so I can't work on this right now but I can get back to it later maybe. |
Description
We're trying to reduce the page size from using the Builder SDK. We've noticed there's about 1mb of component info stored in state and I noticed that the components are essentially stored twice in 2x different properties. The first step to reducing the payload is just using 1x: registeredComponents and passing it into the EnableEditor component to be used (the only place it is used).
Partially addresses #4166 and saves about 500kb of state (depending on component count ofc)
Screenshot
Note
Consolidates component registration by removing
componentInfosfrom context and usingregisteredComponentspassed fromContenttoEnableEditorfor editor registration, with minor version bumps.state.registeredComponentsfrom defaults +customComponents, serialize, and expose viaComponentsContext.registeredComponentstoBlocksand toEnableEditor.EnableEditornow acceptsregisteredComponentsand posts register messages from it; stops readingbuilderContextSignal.value.componentInfos.componentInfosfromBuilderContextInterfaceand default context.componentInfosinjection from React Native context setup inmitosis.config.cjs.@builder.io/sdkand all framework SDKs.Written by Cursor Bugbot for commit 47bee42. This will update automatically on new commits. Configure here.