Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
fix(types): emitted can return undefined #1431
Conversation
8a8950a
to
68249d0
|
After seeing the impact this has on |
|
Did not review the codebase yet, but I don't think Either way, appreciate the effort to making a contribution. What do you think? I can look more into this if there is an |
|
If |
|
( |
|
Right, I get it now. I think this makes sense. Is this modified test code supposed to be there? I think just having the typings is probably fine. If you can remove that, I'd be happy to merge this. |
|
Without the changes to the test code, I got a CI failure because TypeScript complained about values being possibly |
|
Ok, I think this makes sense. Thanks! |

Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.

I’m not sure how strict this project is about
undefinedtypes in general, but I found it confusing that the documentation advised to useexpect(…).toBeTruthy()on a function (emitted( 'foo' )) that was typed to always return an array (i. e., always truthy).It might be better to only add the
|undefinedto the overload with an event name, not to the overload that returns all events at once… not sure.What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
I suppose in theory it’s a breaking change? People who previously assigned the result of.emitted()to a variable with an explicit type might now get a type error. The expected use, inline withinexpect(), should be fine, though.Calls like
.emitted().foo[0]or.emitted('foo')[0]may also need to be updated, depending on TypeScript config – in the tests, I’ve used the non-null assertion operator.The PR fulfills these requirements:
devbranch.fix #xxx[,#xxx], where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information: