Skip to content

Conversation

gabritto
Copy link
Member

@gabritto gabritto commented Jan 4, 2023

Fixes #48796.

The original error elaboration tried to detect whether a component expected multiple children or just one by checking if the types were array-like. However, the type definition of ReactNode, more specifically of its ReactFragment component, were updated to use Iterable<ReactNode>. So that meant that, if a component expected children of type ReactNode (or ReactFragment, we'd think it was expecting a single child, because Iterable<ReactNode> is not an array-like type, so the error message reflected that, leading to TS confusingly saying the component expected a single child even when an array of children of the correct type would be accepted.

I updated that error elaboration to use assignability to Iterable (when this type is defined) as the way to differentiate between components expecting a single child or multiple children. I also had to further change the error elaboration used to account for an use of Iterable.

@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Jan 4, 2023
* Assumes `target` type is assignable to the `Iterable` type, if `Iterable` is defined,
* or that it's an array-like type, if `Iterable` is not defined.
*/
function elaborateIterableOrArrayLikeTargetElementwise(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function was copied from the existing elaborateElementwise function, and then modified to work with a target that is iterable or array-like. Some parts of the elaboration were removed because I figured they didn't apply in this specific case, and some were adapted.

@gabritto gabritto marked this pull request as draft January 4, 2023 21:22
@gabritto gabritto marked this pull request as ready for review January 4, 2023 22:04
@gabritto
Copy link
Member Author

gabritto commented Jan 5, 2023

@typescript-bot perf test this faster
@typescript-bot run dt
@typescript-bot user test this
@typescript-bot test top100

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 5, 2023

Heya @gabritto, I've started to run the parallelized Definitely Typed test suite on this PR at ee4ea51. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 5, 2023

Heya @gabritto, I've started to run the diff-based user code test suite on this PR at ee4ea51. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 5, 2023

Heya @gabritto, I've started to run the diff-based top-repos suite on this PR at ee4ea51. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 5, 2023

Heya @gabritto, I've started to run the abridged perf test suite on this PR at ee4ea51. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

@gabritto Here are the results of running the user test suite comparing main and refs/pull/52105/merge:

Everything looks good!

@typescript-bot
Copy link
Collaborator

@gabritto Here are the results of running the top-repos suite comparing main and refs/pull/52105/merge:

Everything looks good!

@typescript-bot
Copy link
Collaborator

@gabritto
The results of the perf run you requested are in!

Here they are:

Comparison Report - main..52105

Metric main 52105 Delta Best Worst
Angular - node (v16.17.1, x64)
Memory used 372,007k (± 0.01%) 371,989k (± 0.01%) -19k (- 0.00%) 371,954k 372,035k
Parse Time 4.13s (± 0.32%) 4.13s (± 0.41%) +0.00s (+ 0.11%) 4.10s 4.15s
Bind Time 1.27s (± 0.55%) 1.27s (± 0.61%) +0.00s (+ 0.03%) 1.26s 1.28s
Check Time 9.31s (± 0.52%) 9.32s (± 0.63%) +0.01s (+ 0.07%) 9.24s 9.39s
Emit Time 8.03s (± 0.55%) 7.99s (± 0.29%) -0.04s (- 0.55%) 7.96s 8.03s
Total Time 22.74s (± 0.12%) 22.71s (± 0.33%) -0.03s (- 0.13%) 22.62s 22.82s
Compiler-Unions - node (v16.17.1, x64)
Memory used 200,590k (± 0.64%) 200,604k (± 0.70%) +14k (+ 0.01%) 199,996k 203,476k
Parse Time 1.81s (± 0.57%) 1.81s (± 0.65%) +0.00s (+ 0.08%) 1.80s 1.83s
Bind Time 0.84s (± 1.22%) 0.84s (± 0.77%) +0.00s (+ 0.10%) 0.83s 0.85s
Check Time 10.12s (± 1.29%) 10.09s (± 0.75%) -0.03s (- 0.33%) 9.99s 10.22s
Emit Time 2.99s (± 1.21%) 2.98s (± 0.90%) -0.01s (- 0.38%) 2.93s 3.01s
Total Time 15.76s (± 1.06%) 15.73s (± 0.62%) -0.04s (- 0.25%) 15.62s 15.91s
Monaco - node (v16.17.1, x64)
Memory used 353,413k (± 0.01%) 353,407k (± 0.01%) -6k (- 0.00%) 353,386k 353,440k
Parse Time 3.17s (± 0.54%) 3.16s (± 0.33%) -0.01s (- 0.17%) 3.15s 3.18s
Bind Time 1.13s (± 1.34%) 1.12s (± 0.43%) -0.00s (- 0.07%) 1.12s 1.13s
Check Time 7.85s (± 0.45%) 7.87s (± 0.67%) +0.02s (+ 0.25%) 7.80s 7.96s
Emit Time 4.49s (± 0.49%) 4.48s (± 0.40%) -0.02s (- 0.36%) 4.45s 4.50s
Total Time 16.63s (± 0.18%) 16.63s (± 0.45%) -0.00s (- 0.01%) 16.52s 16.75s
TFS - node (v16.17.1, x64)
Memory used 309,368k (± 0.00%) 309,364k (± 0.00%) -4k (- 0.00%) 309,357k 309,372k
Parse Time 2.60s (± 1.08%) 2.63s (± 1.67%) +0.02s (+ 0.90%) 2.56s 2.68s
Bind Time 1.07s (± 2.18%) 1.05s (± 1.49%) -0.02s (- 1.64%) 1.04s 1.08s
Check Time 7.39s (± 0.50%) 7.38s (± 0.26%) -0.01s (- 0.09%) 7.36s 7.41s
Emit Time 4.19s (± 0.49%) 4.21s (± 0.87%) +0.01s (+ 0.34%) 4.15s 4.24s
Total Time 15.26s (± 0.33%) 15.27s (± 0.55%) +0.02s (+ 0.12%) 15.12s 15.36s
material-ui - node (v16.17.1, x64)
Memory used 484,506k (± 0.00%) 484,552k (± 0.00%) +46k (+ 0.01%) 484,517k 484,570k
Parse Time 3.67s (± 0.35%) 3.68s (± 0.54%) +0.01s (+ 0.16%) 3.66s 3.71s
Bind Time 1.01s (± 0.47%) 1.02s (± 0.27%) +0.01s (+ 0.73%) 1.01s 1.02s
Check Time 17.85s (± 0.25%) 17.95s (± 0.65%) +0.10s (+ 0.58%) 17.82s 18.14s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 22.54s (± 0.22%) 22.65s (± 0.56%) +0.11s (+ 0.51%) 22.50s 22.84s
xstate - node (v16.17.1, x64)
Memory used 567,851k (± 0.02%) 567,864k (± 0.02%) +13k (+ 0.00%) 567,757k 567,970k
Parse Time 4.75s (± 0.34%) 4.76s (± 0.15%) +0.01s (+ 0.27%) 4.75s 4.77s
Bind Time 1.65s (± 0.61%) 1.66s (± 0.45%) +0.01s (+ 0.70%) 1.65s 1.67s
Check Time 2.83s (± 0.51%) 2.82s (± 0.35%) -0.01s (- 0.33%) 2.81s 2.83s
Emit Time 0.09s (± 0.16%) 0.09s (± 5.83%) 🟩-0.00s (- 3.52%) 0.08s 0.09s
Total Time 9.31s (± 0.30%) 9.34s (± 0.19%) +0.03s (+ 0.29%) 9.31s 9.36s
System
Machine Namets-ci-ubuntu
Platformlinux 5.4.0-135-generic
Architecturex64
Available Memory16 GB
Available Memory15 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v16.17.1, x64)
Scenarios
  • Angular - node (v16.17.1, x64)
  • Compiler-Unions - node (v16.17.1, x64)
  • Monaco - node (v16.17.1, x64)
  • TFS - node (v16.17.1, x64)
  • material-ui - node (v16.17.1, x64)
  • xstate - node (v16.17.1, x64)
Benchmark Name Iterations
Current 52105 6
Baseline main 6

Developer Information:

Download Benchmark

@gabritto gabritto merged commit e60c210 into main Jan 6, 2023
@gabritto gabritto deleted the gabritto/issue48796 branch January 6, 2023 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unintuitive error message when some JSX children is invalid
3 participants