Skip to content

[content] Fix build fail error related to non string styles params#492

Open
MenKNas wants to merge 6 commits into
devfrom
bugfix/JSS-9256-non-string-styles-params
Open

[content] Fix build fail error related to non string styles params#492
MenKNas wants to merge 6 commits into
devfrom
bugfix/JSS-9256-non-string-styles-params

Conversation

@MenKNas
Copy link
Copy Markdown
Contributor

@MenKNas MenKNas commented May 28, 2026

Description / Motivation

This PR provides a solution fix for a prerender build crash caused by calling .match() on non-string rendering params when DetailedRenderingParams returns objects.

This is done by introducing a small asString guard in themes traversal so regex logic runs only on real strings, while non-string values are safely ignored and existing fallback behaviour remains intact.

Testing Details

  • Unit Test Added
  • Manual Test/Other (Please elaborate)

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)

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 28, 2026

🦋 Changeset detected

Latest commit: 7375070

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

This PR includes changesets to release 1 package
Name Type
@sitecore-content-sdk/content 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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 28, 2026

📦 Package Size and Test Coverage Report

Package Base Size PR Size Δ Change Base Coverage PR Coverage Δ Change
analytics-core 67.34 KB 67.34 KB ✅ 0.00 KB 97.40% 97.40% 0.00%
cli 59.14 KB 59.14 KB ✅ 0.00 KB 70.09% 70.09% 0.00%
content 429.03 KB 433.90 KB 🔺 +4.87 KB 92.38% 92.46% +0.08%
core 120.61 KB 120.61 KB ✅ 0.00 KB 91.27% 91.27% 0.00%
create-content-sdk-app 542.41 KB 542.41 KB ✅ 0.00 KB 96.35% 96.35% 0.00%
events 72.27 KB 72.27 KB ✅ 0.00 KB 97.97% 97.97% 0.00%
nextjs 568.35 KB 568.35 KB ✅ 0.00 KB 90.97% 90.97% 0.00%
personalize 64.01 KB 64.01 KB ✅ 0.00 KB 99.74% 99.74% 0.00%
react 230.78 KB 231.36 KB 🔺 +0.58 KB 93.80% 93.82% +0.02%
search 8.10 KB 8.10 KB ✅ 0.00 KB 98.57% 98.57% 0.00%
Total 🔺 +5.45 KB

@MenKNas MenKNas requested a review from illiakovalenko May 28, 2026 12:43
Copy link
Copy Markdown
Collaborator

@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.

I think we’re missing a couple of scenarios here, which could introduce bugs. It would be good to test these cases locally as well.

  1. There is a scenario where styles can be provided as an object, but its value (Styles.value) is currently ignored. If this is part of the defined contract, we should validate and properly handle the value; otherwise customers will not be able to apply styles as expected.
Image
  1. I assigned a styles id to the component in Pages Editor and also added additional styles via the right-hand side panel (e.g. alignment, classname). In this case, we receive a merged JSON including the styles id, which is incorrect and should be addressed - likely by a different team - likely it should be done before our release, otherwise customers will report a new item, please create an item / test and track it. Currently we just set a huge string as a class.
Image

@MenKNas
Copy link
Copy Markdown
Contributor Author

MenKNas commented Jun 3, 2026

I think we’re missing a couple of scenarios here, which could introduce bugs. It would be good to test these cases locally as well.

  1. There is a scenario where styles can be provided as an object, but its value (Styles.value) is currently ignored. If this is part of the defined contract, we should validate and properly handle the value; otherwise customers will not be able to apply styles as expected.
Image 2. I assigned a `styles` id to the component in Pages Editor and also added additional styles via the right-hand side panel (e.g. alignment, classname). In this case, we receive a merged JSON including the `styles` id, which is incorrect and should be addressed - likely by a different team - likely it should be done before our release, otherwise customers will report a new item, please create an item / test and track it. Currently we just set a huge string as a class. Image

@illiakovalenko

First comment is valid, I have addressed it in order to cover the scenario of styles as an object.

On the second comment, this is inherently wrong. Styles should be a plain class string or a proper object, not a stringified JSON blob. This needs to be fixed on the Pages/Layout Service side.

@MenKNas MenKNas requested a review from illiakovalenko June 3, 2026 14:49
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