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

useComponentProps returns an error object instead of data #1495

Closed thany closed 3 months ago

thany commented 1 year ago

Describe the Bug

The return value of useComponentProps may be the actual data, or an error object. This is bad for typescript, since the generics parameter of useComponentProps dictates a specific return type. This return type is no longer respected when an error is thrown during fetching. Then, the return type is a { error: string } type.

The only way to deal with this right now, is to use ugly ducktyping. Ugly because I am forced to disrepect my own typings there momentarily.

To Reproduce

Use useComponentProps and throw an error in fetching data.

Expected Behavior

The common way to solve this is for a hook to return two values: the data (if available), and an error object. Like so:

const [ data, error ] = useComponentProps<TData>( ... );

The signature of useComponentProps then being something like:

function useComponentProps<ComponentData>(componentUid: string | undefined): [ComponentData | undefined, { error: string }];

That way both the data and an error, should one occur, are kept separate and strongly typed.

Possible Fix

No response

Provide environment information

thany commented 1 year ago

You could also take a look at swr for one of numerous React hooks examples that return two different values. Heck, even useState is a perfectly good example of this.

thany commented 1 year ago

Here's a full workaround:

import { useComponentProps as useComponentPropsSitecore } from '@sitecore-jss/sitecore-jss-nextjs';

export const useComponentProps = <TData>(componentUid: string | undefined): [TData | undefined, string | undefined] => {
  const data = useComponentPropsSitecore<TData | { error: string }>(componentUid);
  if (data && typeof data === 'object' && 'error' in data && typeof data.error === 'string') {
    return [undefined, data.error];
  }

  return [data as TData, undefined];
};

This is a horrible solution (hence I call it a workaround) and the proper solution is if JSS wouldn't blatantly put the wrong data type into the rendering in the first place. So DO NOT copy this over to the actual JSS code, but instead make it so that an error and data are always kept separate while passing data around.

art-alexeyenko commented 1 year ago

Thanks for the report @thany . I've put this in our backlog.

thany commented 1 year ago

What does that mean? Where can we see this backlog? I don't think Github has anything like that...

illiakovalenko commented 1 year ago

@thany We have our internal backlog where we collect GitHub issues/features and prioritize that accordingly

stale[bot] commented 4 months ago

This has been automatically marked as stale because it has not had recent activity. It will be closed if there is no further activity within 30 days. You may add comments or the 'keep' label to prevent it from closing. Thank you for your contributions.