type fixes for React 18 #20575
type fixes for React 18 #20575
Conversation
Rugvip
requested review from
a team,
kuangp,
alde and
backstage-service
as code owners
github-actions
bot
added
area:catalog
This comment was marked as resolved.
This comment was marked as resolved.
Rugvip
force-pushed
the
rugvip/react18
branch
from
eed7d3f
to
dba09cb
Compare
|
Uffizzi Preview |
Rugvip
force-pushed
the
rugvip/react18
branch
from
dba09cb
to
ec770f7
Compare
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Rugvip
force-pushed
the
rugvip/react18
branch
from
ec770f7
to
0296f27
Compare
|
Alright, changesets added for all changes that aren't just internal types, this should be ready to go now |
| @@ -249,7 +249,7 @@ export interface DependencyGraphProps<NodeData, EdgeData> | |||
| acyclicer?: 'greedy'; | |||
| align?: DependencyGraphTypes.Alignment; | |||
| curve?: 'curveStepBefore' | 'curveMonotoneX'; | |||
| defs?: SVGDefsElement | SVGDefsElement[]; | |||
| defs?: JSX.Element | JSX.Element[]; | |||
There was a problem hiding this comment.
Wondering a bit about this as it seems to make a specific type more general. Did you use this to prevent a ... cannot be used as a JSX component error? Saw this comment, which might be helpful: software-mansion/react-native-svg#1741 (comment)
There was a problem hiding this comment.
Nah SVGDefsElement is just a wildly wrong type here, it's an actual DOM element, which you can't use in React in the way that the below code does. You also can't narrow down element types with TypeScript, so JSX.Element + docs it is x)
| @@ -181,7 +181,9 @@ export const FileExplorer = () => { | |||
| } | |||
| if (!value) { | |||
| return ( | |||
| <Alert severity="warning">No code coverage found for ${entity}</Alert> | |||
| <Alert severity="warning"> | |||
| No code coverage found for {humanizeEntityRef(entity)} | |||
| {({ results }) => ( | ||
| <> | ||
| {results.map((result, index) => { |
There was a problem hiding this comment.
Can you maybe explain this change a bit more? A bit confused why the wrapping Fragment is required :/
There was a problem hiding this comment.
The (resultSet: SearchResultSet) => JSX.Element type doesn't allow for JSX.Element[], so we wrap it in a fragment to return just a single element.
Might be worth changing SearchResultProps instead though, @camilaibs? 馃榿
There was a problem hiding this comment.
On second thought I'd rather make the type change in a followup, trying to keep actual changes in this one to a minimum 馃槄
| return ( | ||
| <SearchBarBase | ||
| value={query} | ||
| onSubmit={handleSubmit} | ||
| onChange={handleChange} | ||
| onChange={setQuery} |
|
Thank you for contributing to Backstage! The changes in this pull request will be part of the |
馃Ч, couple of fixes to make types work for both React 17 and 18
Work towards #12252
adding changesets once things build 馃槄