-
Notifications
You must be signed in to change notification settings - Fork 950
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
fix[gen1][core] ENG-7515 add query.id when hitting content API for symbols #3752
base: main
Are you sure you want to change the base?
Conversation
|
packages/core/src/builder.class.ts
Outdated
if (apiEndpoint === 'content' && queue[0].model === 'symbol') { | ||
queryParams['query.id'] = queue[0].entry; | ||
} | ||
|
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.
We should set this in Symbol.tsx
instead. I think it can go in here:
builder/packages/react/src/blocks/Symbol.tsx
Lines 159 to 165 in bff6858
options={{ | |
...(!this.isEditingThisSymbol && { | |
key: builderComponentKey, | |
noEditorUpdates: true, | |
}), | |
}} | |
codegen={!!content?.data?.blocksJs} |
The reason for that is because the model name might be something other than symbol
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.
The change is working as expected. Here is a loom with a demo of the changes https://www.loom.com/share/083d6711f94048ce8b88c514c71012e4
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.
the query param is in a condition it shouldn't be in
@@ -160,6 +160,9 @@ class SymbolComponent extends React.Component<PropsWithChildren<SymbolProps>> { | |||
...(!this.isEditingThisSymbol && { | |||
key: builderComponentKey, | |||
noEditorUpdates: true, | |||
query: { |
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.
this query param should not be condition in editingThisSymbol. it should always be provided
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.
also, this query param should only be added when the entry value exists AND when the apiEndpoint value is "content"
Description
In Gen1 SDK, when content API is hit for Symbols, it does not render symbols properly
Jira ticket
https://builder-io.atlassian.net/browse/ENG-7515
Loom
https://www.loom.com/share/90d3b6e317bb493e98ad418ac2f0b7a0