don't needed crash if value is empty

[xam8re]

Fixes #
Fix crash if an empty attribute is passed to a component.

Proposed Changes

Added a optional param that represent the empty value.
If empty value is passed, this parameter is used. Default as empty string.

[czernika]

Does this really fix it? The error comes from constructor because of incorrect type. If just change type into protected ?float $value this should fix it (tested only on PHP8.2)

Same should be valid for other cells.

However I can see value in empty parameter. null and 0 are different parameters and show them both as 0.00 may be not a correct solution.

But instead of cell param how about TD method? Something like

TD::make('total', 'Total')
  ->usingComponent(Currency::class, after: '$') // or even without using component
  ->empty('Account was not set'),

If bank account was not set and total value is null it will show 'Account was not set'. But if user spent all of his money and his bank account is literally 0 - it will show 0.00 $

Plus if the value is null there is really no point in rendering component. We can return early before rendering column if value is null. This way we can keep correct types for cell components without null (which makes more sense for me as Currency and other typed cells should process some value - they should not handle 'no value' situation)

[xam8re]

Sometime who develop frontend/backend have no clue on database. In my case, price column is null. Actually there only 2 choice:
no formatting
let developer to manage quickly a null case.
Let the developer choose best way but, crash is not a good solution IMHO.

[czernika]

@xam8re I'm not saying this is good. In fact it is not application-level exception but PHP TypeError because you're passing null value into constructor which accepts float only as value. It needs to be fixed of course. My point is how to fix it, which is better way

I don't think your solution would work as it comes after value being passed into class. I'm basically suggesting this solution instead (simplified dummy code)

if (is_null($value)) {
  return $empty;
}

return new Currency($value);

Do not pass null into currency cell (etc), but handle this case even before rendering (not within Currency class)

[czernika]

To be more concrete here is my suggestion

Add new property and method into Orchid\Screen\TD class

protected $empty = '';

public function empty($empty)
{
  $this->empty = $empty;

  return $this;
}

Change a little buildTd method

- 'value'   => $value,
+ 'value'   => $value ?? $this->empty,

And renderComponent of Cell

protected function renderComponent(string $component, $value, array $params = []): ?string
{
+  if (is_null($value)) {
+    return $this->empty;
+  }

  [$class, $view] = Blade::componentInfo($component);

This way:

1. We kept cell components strict types
2. Prevent it from crashes when null value is being passed (it doesn't even get to cell component)
3. Add extra flexibility for developer (if there is a null value empty string will be shown instead. But they are free to change it with whatever value they wish via empty() method regardless of using you component or not)

Big question for me is what to be considered empty (for now this will check null value only, but when value is empty string '' - it is empty as well. Should we handle this case or null only? If not name empty will be confusing)

Or

Just change strict float type of Currency component (and every other) into ?float (null or float) - fixes error, leave everything else as it is, no extra feature (which could be considered as redundant)

[xam8re]

Yes. Good suggestion. I pr this only because I needed a quick fix. I'm using orchid only from 1 week. I implement your suggestion and re submit it to pr. Thanks

[czernika]

@xam8re If you need quick fix in a project you may render component itself, e.g

// this code part is from my project with exact same problem but for Number component
// Instead I'm using Laravel's Number helper and render everything on my own

TD::make('star_avg_rate', __('Average rate'))
  ->render(fn (Star $star) => Number::format($star->ranking->points ?? 0, 2)),

or use Laravel accessor for attribute to ensure it will return float

[murufi]

Very good insight