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

fix[gen2][builderStudio] ENG-7646 add Builder Studio support for Gen-2 SDK #3794

Merged
merged 9 commits into from
Jan 13, 2025

Conversation

clyde-builderio
Copy link
Contributor

@clyde-builderio clyde-builderio commented Dec 20, 2024

Description

Builder Components not rendering in Studio Tab on Gen 2 SDK

Jira Ticket
https://builder-io.atlassian.net/browse/ENG-7646

Jira Ticket for Gen1 References
https://builder-io.atlassian.net/browse/ENG-4980

Loom
https://www.loom.com/share/063d068f6be84b54adb027b03ec14793

Loom where current changes will not give desired result
https://www.loom.com/share/6628e8caa98f4af2905abb2c6c0d4646

Copy link

changeset-bot bot commented Dec 20, 2024

🦋 Changeset detected

Latest commit: 73bca88

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
@builder.io/sdk-angular Patch
@builder.io/sdk-react-nextjs Patch
@builder.io/sdk-qwik Patch
@builder.io/sdk-react Patch
@builder.io/sdk-react-native Patch
@builder.io/sdk-solid Patch
@builder.io/sdk-svelte Patch
@builder.io/sdk-vue Patch

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

Copy link

nx-cloud bot commented Dec 20, 2024

View your CI Pipeline Execution ↗ for commit 73bca88.

Command Status Duration Result
nx test @e2e/nextjs-sdk-next-app ✅ Succeeded 7m 31s View ↗
nx test @e2e/nuxt ✅ Succeeded 7m 7s View ↗
nx test @e2e/qwik-city ✅ Succeeded 7m 5s View ↗
nx test @e2e/react-native ✅ Succeeded 5m 36s View ↗
nx test @e2e/react-sdk-next-15-app ✅ Succeeded 5m 7s View ↗
nx test @e2e/sveltekit ✅ Succeeded 5m 7s View ↗
nx test @e2e/react-sdk-next-14-app ✅ Succeeded 4m 51s View ↗
nx test @e2e/react-sdk-next-pages ✅ Succeeded 4m 49s View ↗
Additional runs (34) ✅ Succeeded ... View ↗

☁️ Nx Cloud last updated this comment at 2025-01-10 15:19:06 UTC

Copy link

gitguardian bot commented Jan 9, 2025

⚠️ GitGuardian has uncovered 2 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

Since your pull request originates from a forked repository, GitGuardian is not able to associate the secrets uncovered with secret incidents on your GitGuardian dashboard.
Skipping this check run and merging your pull request will create secret incidents on your GitGuardian dashboard.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
14200383 Triggered Generic High Entropy Secret 22da0de packages/core/src/builder.class.test.ts View secret
14200385 Triggered Generic High Entropy Secret 22da0de packages/core/src/builder.class.test.ts View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Comment on lines +139 to +140
queryOptions['userAttributes.urlPath'] = window.location.pathname;
queryOptions['userAttributes.host'] = window.location.host;
Copy link
Contributor

Choose a reason for hiding this comment

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

is this how we handle on gen1 as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's a loom explaining how it's happening on Gen1 SDK https://www.loom.com/share/83bd96e1d6a743639e67165c272765df

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for explaining!

Comment on lines 327 to 329
...(searchParamPreviewModel === 'BUILDER_STUDIO' && props.content
? { query: { id: props.content?.id } }
: {}),
Copy link
Contributor

Choose a reason for hiding this comment

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

could you please explain why this is necessary?

Copy link
Contributor Author

@clyde-builderio clyde-builderio Jan 10, 2025

Choose a reason for hiding this comment

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

for Symbols we need to pass query.id. So for that purpose I have added this here. This PR will provide the reference https://github.com/BuilderIO/builder/pull/3752/files#diff-f1e59e260dae927ec6d6a87239e3f224080638f90af79871855dc8b5afead472

Copy link
Contributor

Choose a reason for hiding this comment

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

should we pass this from the Symbol level then? or I believe context has symbolId (https://github.com/sidmohanty11/builder/blob/main/packages/sdks/src/blocks/symbol/symbol.lite.tsx/#L136)? just to ensure that query is not passed when Content is not a symbol?

Copy link
Contributor Author

@clyde-builderio clyde-builderio Jan 10, 2025

Choose a reason for hiding this comment

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

We can replace

...(searchParamPreviewModel === 'BUILDER_STUDIO' && props.content
            ? { query: { id: props.content?.id } }
            : {}),

with

...(searchParamPreviewModel === 'BUILDER_STUDIO' &&
          props.context?.symbolId
            ? { query: { id: props.context.symbolId } }
            : {}),

Good catch! @sidmohanty11

@clyde-builderio clyde-builderio merged commit e0dc757 into BuilderIO:main Jan 13, 2025
50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants