LabeledValue is throwing for a null value

[elodierafali]

Provide a general summary of the issue here

<LabeledValue value={null} /> is throwing this error:

TypeError: Cannot use 'in' operator to search for 'start' in null
 at $3e9971be431adb24$var$LabeledValue (LabeledValue.tsx:85:1)

🤔 Expected Behavior?

<LabeledValue value={null} /> should not throw

😯 Current Behavior

The app page crashes

💁 Possible Solution

In the code do not use typeof value === 'object' because it's true for null or verify value is not null.

🔦 Context

We use LabeledValue with values that can be set to null by our third party form engine so it's difficult to make an exception just for this case to have the form engine not set null values when there is no value.

🖥️ Steps to Reproduce

Try <LabeledValue value={null} />

Version

3.22.0

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

MacOS Monterey

🧢 Your Company/Team

Adobe Journey Optimizer

🕷 Tracking Issue

No response

[reidbarber]

Looks like we should add a case for

  if (value == null) {
    children = 'null';
  }

above this line

react-spectrum/packages/@react-spectrum/labeledvalue/src/LabeledValue.tsx

Lines 85 to 87 in 3276459

	if (typeof value === 'object' && 'start' in value && typeof value.start === 'number' && typeof value.end === 'number') {
	children = <FormattedNumber value={value as NumberValue} formatOptions={formatOptions as Intl.NumberFormatOptions} />;
	}

and then add value != null && guards before every typeof value === 'object'.

Assuming we want to render the string 'null' in this case.

[elodierafali]

Looks like we should add a case for

  if (value == null) {
    children = 'null';
  }

above this line

react-spectrum/packages/@react-spectrum/labeledvalue/src/LabeledValue.tsx

Lines 85 to 87 in 3276459

	if (typeof value === 'object' && 'start' in value && typeof value.start === 'number' && typeof value.end === 'number') {
	children = <FormattedNumber value={value as NumberValue} formatOptions={formatOptions as Intl.NumberFormatOptions} />;
	}

and then add value != null && guards before every typeof value === 'object'.

Assuming we want to render the string 'null' in this case.

But in our case null is like empty string or undefined we don't want to display "null" in the component.

[reidbarber]

What would you expect to be rendered? I think the string null is a pretty safe bet if the value is null. I think an empty string might be confusing, and something like - would be confusing to screen reader users.

You can always handle these cases yourself and transform it into something that makes sense for your use case before it gets rendered by the component.

[snowystinger]

I think re-using 'None' from Picker would be better. 'null' is quite specific to us javascript programmers. To the end user, I think saying 'None' would make more sense, as it communicates there is no value.

Otherwise, I'd suggest using a string value instead https://react-spectrum.adobe.com/react-spectrum/LabeledValue.html#label-alignment-and-position and providing the translated string you want for a particular type of empty value

[elodierafali]

We do not intend to put null in the LabeledValue but the form engine might put null as a value as the value is dynamic. So what we intend is a string value but the input is dynamic and can end up being null or a non-controlled value as we use it like this <LabeledValue value={dynamicValue}/> and not like <LabeledValue value="translatedValue"/>. So the null value is not something we put on purpose, and is equivalent to undefined for us. So we would expect it to render like undefined which does not throw and outputs an empty string.

[devongovett]

Couldn't you do something like <LabeledValue value={dynamicValue ?? ''} />?

[elodierafali]

Couldn't you do something like <LabeledValue value={dynamicValue ?? ''} />?

Yes we can do that as a workaround, but it seems better to fix it in the component to not have the component crash for a null value when it doesn't crash for undefined. In the stack trace it is not obvious the crash is because of null, I think it will be difficult for teams to trace null value is in fact not valid. Maybe you can add an error message or make it just a console.error to not have the component crash at least.

[devongovett]

To be fair, null is not documented as a valid value (and errors in TypeScript strict mode at least), and it's not obvious what we would render in case null is passed. You'd likely want to control that anyway, and it's easy to do that already.

[elodierafali]

We had already planned to add a control since we have to prevent our app from crashing because of this. But I thought this should be reported as it seemed like the real fix for the long term to not have the component crash. In our stack the component didn't error out and we saw the crash in prod.

Why is it not ok to add a check on the value before typeof value === 'object' to exclude null, isn't this a bug? and for the rendering why is it not ok that it renders like undefined or empty string when it's null?