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.

Sign up for GitHub

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

Jump to bottom

[nextjs] Bring your own code (BYOC) feature #1568

Merged
merged 16 commits into from
Jul 26, 2023
Merged

[nextjs] Bring your own code (BYOC) feature #1568

merged 16 commits into from
Jul 26, 2023

Conversation

art-alexeyenko
Copy link
Contributor

@art-alexeyenko art-alexeyenko commented Jul 20, 2023 •
edited
Loading

Description / Motivation

BYOC unwrapped allows developers to create components and then use them without following the component deployment guidelines. It means that components:

  • Can use any prop type, without consideration for layout service data
  • Be located anywhere
  • Are not required to be deployed to Sitecore as rendering items

Right now BYOC can be used with Component Builder, but this PR allows using registered components on a page, directly.

Minor side effect of this PR is changes to MissingComponent - it can now display custom error messages to accommodate BYOC usecases.

Testing Details

  • Unit Test Added
  • Manual Test/Other (Please elaborate) - tested in connected mode with and without providing props to inner component

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@art-alexeyenko art-alexeyenko requested review from Inviz and a team July 20, 2023 20:37
addy-pathania
addy-pathania requested changes Jul 20, 2023
View reviewed changes
Copy link
Contributor

@addy-pathania addy-pathania left a comment

Choose a reason for hiding this comment

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

Looks Great. Added one minor suggestion.

packages/sitecore-jss-react/src/components/BYOCRenderer.tsx Outdated Show resolved Hide resolved
ambrauer
ambrauer reviewed Jul 21, 2023
View reviewed changes
Copy link
Contributor

@ambrauer ambrauer left a comment

Choose a reason for hiding this comment

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

Does this solution account for error handling within Sitecore editors (since we know React error boundaries do not work in Next.js/SSR)? For example, if there is an error in a BYOC component, will it be isolated to the component or affect the entire page?

packages/create-sitecore-jss/src/templates/nextjs-sxa/src/components/BYOCWrapper.tsx Outdated Show resolved Hide resolved
illiakovalenko
illiakovalenko requested changes Jul 24, 2023
View reviewed changes
Copy link
Contributor

@illiakovalenko illiakovalenko left a comment

Choose a reason for hiding this comment

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

Looks great 👍
See some minor suggestions below

packages/sitecore-jss-react/src/components/BYOCRenderer.tsx Outdated Show resolved Hide resolved
packages/sitecore-jss-react/src/utils.ts Show resolved Hide resolved
ambrauer
ambrauer suggested changes Jul 24, 2023
View reviewed changes
Copy link
Contributor

@ambrauer ambrauer left a comment

Choose a reason for hiding this comment

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

Looking really good. Just a few things noticed related to error handling.

packages/sitecore-jss-react/src/components/BYOCRenderer.tsx Outdated Show resolved Hide resolved
packages/sitecore-jss-react/src/components/BYOCRenderer.tsx Outdated Show resolved Hide resolved
packages/sitecore-jss-react/src/components/BYOCRenderer.tsx Outdated Show resolved Hide resolved
@art-alexeyenko art-alexeyenko requested a review from ambrauer July 25, 2023 21:46
ambrauer
ambrauer approved these changes Jul 25, 2023
View reviewed changes
Copy link
Contributor

@ambrauer ambrauer left a comment

Choose a reason for hiding this comment

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

Just a couple suggestions, otherwise this looks great!

packages/sitecore-jss-react/src/components/BYOCRenderer.tsx Outdated Show resolved Hide resolved
packages/sitecore-jss-react/src/components/BYOCRenderer.tsx Outdated Show resolved Hide resolved
@art-alexeyenko art-alexeyenko changed the title [nextjs] Bring your own components (BYOC) feature [nextjs] Bring your own code (BYOC) feature Jul 26, 2023
addy-pathania
addy-pathania approved these changes Jul 26, 2023
View reviewed changes
Copy link
Contributor

@addy-pathania addy-pathania left a comment

Choose a reason for hiding this comment

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

👍🏼

illiakovalenko
illiakovalenko approved these changes Jul 26, 2023
View reviewed changes
Copy link
Contributor

@illiakovalenko illiakovalenko left a comment

Choose a reason for hiding this comment

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

👍

@art-alexeyenko art-alexeyenko merged commit e5d07f7 into dev Jul 26, 2023
@art-alexeyenko art-alexeyenko deleted the feature/jss-60-byoc-productize branch July 26, 2023 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@addy-pathania addy-pathania addy-pathania approved these changes

@illiakovalenko illiakovalenko illiakovalenko approved these changes

@ambrauer ambrauer ambrauer approved these changes

@Inviz Inviz Awaiting requested review from Inviz

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@art-alexeyenko @ambrauer @illiakovalenko @addy-pathania