Sitecore / jss

Software development kit for JavaScript developers building web applications with Sitecore Experience Platform
https://jss.sitecore.com
Apache License 2.0
261 stars 275 forks source link

withPlaceholder crashes when the rendering doesn't have any rendering parameters #1222

Closed Elyseum closed 1 year ago

Elyseum commented 2 years ago

Description

After upgrading to v20.1.3 on Sitecore 10.2 (fetched with REST), most of my container components (using the withPlaceholder function) started failing with a "TypeError: Cannot read properties of undefined (reading 'FieldNames')" error message in WithPlaceholder.getSXAParams.

In https://github.com/Sitecore/jss/pull/931/files#diff-b11d6af9db06b110eca927815c914ac5574cec4c7fdb45a3710a2c67f5b82c27 this function was added to support SXA styling params on containers.

The implementation assumes params is always present on rendering, which is not the case: it's not defined if the rendering doesn't have any param values. The type indicates the value can be missing: https://github.com/Sitecore/jss/blob/3724280e0d6fe3c344c10f0450296203bf3627a8/packages/sitecore-jss/src/layout/models.ts#L91

Expected behavior

Renderings without params should render as expected.

Possible Fix

In the getSXAParams function, add an extra check to see if the params property is defined or not. This already happens in getComponentForRendering.

Awaiting for a permanent fix, I've implemented following code as a workaround (make sure params is always defined)

withPlaceholder(placeholder, {
    resolvePlaceholderDataFromProps: ({ rendering }: PlaceholderComponentProps) => {
      const { placeholders: placeholdersData } = rendering;
      if (placeholdersData) {
        Object.keys(placeholdersData).forEach((k) => {
          placeholdersData[k]
            .map((c) => c as ComponentRendering)
            .filter((c) => c && !c.params)
            .forEach((c) => (c.params = {}));
        });
      }
      return rendering;
    },

Your Environment

ambrauer commented 2 years ago

@Elyseum Thanks for the information, I've added this to our backlog to investigate / fix in a future release.

We also gladly accept pull requests if you'd like to take a stab at it. Check the contributing guide and we'll be happy to merge it in.