chore: Safe typing for attribute editor

[connorlanigan]

Description

This PR removes unsafe usages of the any type from the Attribute Editor component's folder. It also enables an ESLint rule to prevent future unsafe usages there.

Related links, issue #, if available: n/a

How has this been tested?

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

• Changes include appropriate documentation updates.
• Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
• Changes do not include unsupported browser features, see CONTRIBUTING.md.
• Changes were manually tested for accessibility, see accessibility guidelines.

Security

• If the code handles URLs: all URLs are validated through the checkSafeUrl function.

Testing

• Changes are covered with new/existing unit tests?
• Changes are covered with new/existing integration tests?

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (4df7f8d) 93.98% compared to head (16558d9) 93.98%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1517   +/-   ##
=======================================
  Coverage   93.98%   93.98%           
=======================================
  Files         642      642           
  Lines       17334    17338    +4     
  Branches     5695     5695           
=======================================
+ Hits        16291    16295    +4     
  Misses        973      973           
  Partials       70       70           

Files Changed	Coverage Δ
src/attribute-editor/row.tsx	97.05% <100.00%> (+0.39%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[just-boris]

What is happening here? Isn't typescript by itself can figure that this is a function? (from typeof slot === 'function')

[just-boris]

From the PR description

This PR removes unsafe usages of the any type

But I do not see any removed any's in this diff...

[connorlanigan]

From typeof slot === 'function', Typescript infers the type as (item: T, itemIndex: number) => any. I could not find out precisely why it ends up with that type, but it is caused by the presence of React.ReactNode in line 35.

While the any type of the returned value of this function is not directly visible in the code, the rule still detects its implicit presence and prevents us from returning an any-typed value. With this new code, the return type of the render function is now React.ReactNode.

[just-boris]

In this case I would rather cast the return value than the function:

return slot(item, itemIndex) as React.ReactNode

[connorlanigan]

If we assert the type of the return value, we have to keep that type assertion in sync with the definition of the return type of FieldRenderable, which sounds more risky to me than keeping only FieldRenderable in sync only across line 35 and line 38

[just-boris]

This is unsafe either way. We are not fixing any errors, just making the linter happy (which is another big discussion if these rules actually useful).

I prefer the shorter option because it is more readable, cannot see any runtime hazards, really.

---

When looking into this PR I also learned where this strange any is coming from. ReactNode types are incorrect in our version. Related PR DefinitelyTyped/DefinitelyTyped#56210, "Remove {} from ReactFragment" is the change we need

[just-boris]

Alternatively, we can write a separate typeguard function

function isSlotFunction(slot: unknown) slot is AttributeEditorProps.FieldRenderable<T> {
   return typeof slot == 'function'
}

...

if (isSlotFunction(slot)) {
  return slot(item, itemIndex);
}
return slot;

This is also safe and readable to my standards