[react-draft-wysiwyg] Fix tests assuming instances in ref callbacks are non-nullable#58958
Conversation
|
Inspecting the JavaScript source for this package found some properties that are not in the .d.ts files. yeoman-environment (unpkg)was missing the following properties:
|
|
@eps1lon Thank you for submitting this PR! This is a live comment which I will keep updated. 1 package in this PRCode ReviewsBecause you edited one package and there were no type definition changes, I can help you merge this PR once someone else signs off on it. You can test the changes of this PR in the Playground. Status
All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes. Diagnostic Information: What the bot saw about this PR{
"type": "info",
"now": "-",
"pr_number": 58958,
"author": "eps1lon",
"headCommitOid": "19e4a1a4f717bc2db2c93034f6c9e1d303c3318f",
"mergeBaseOid": "322b09e7e97422bbeb564159b9ffaafdd1c1827f",
"lastPushDate": "2022-02-24T18:17:50.000Z",
"lastActivityDate": "2022-03-16T07:13:33.000Z",
"mergeOfferDate": "2022-03-15T20:27:29.000Z",
"mergeRequestDate": "2022-03-16T07:13:33.000Z",
"mergeRequestUser": "eps1lon",
"hasMergeConflict": false,
"isFirstContribution": false,
"tooManyFiles": false,
"hugeChange": false,
"popularityLevel": "Popular",
"pkgInfo": [
{
"name": "react-draft-wysiwyg",
"kind": "edit",
"files": [
{
"path": "types/react-draft-wysiwyg/test/basic-tests.tsx",
"kind": "test"
}
],
"owners": [
"imechZhangLY",
"brunoMaurice",
"ldanet",
"MunifTanjim",
"n-zeplo"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Popular"
}
],
"reviews": [
{
"type": "approved",
"reviewer": "peterblazejewicz",
"date": "2022-03-15T20:26:49.000Z",
"isMaintainer": true
}
],
"mainBotCommentID": 1050147616,
"ciResult": "pass"
} |
|
🔔 @imechZhangLY @brunoMaurice @ldanet @MunifTanjim @n-zeplo — please review this PR in the next few days. Be sure to explicitly select |
|
Re-ping @imechZhangLY, @brunoMaurice, @ldanet, @MunifTanjim, @n-zeplo: This PR has been out for over a week, yet I haven't seen any reviews. Could someone please give it some attention? Thanks! |
|
It has been more than two weeks and this PR still has no reviews. I'll bump it to the DT maintainer queue. Thank you for your patience, @eps1lon. (Ping @imechZhangLY, @brunoMaurice, @ldanet, @MunifTanjim, @n-zeplo.) |
peterblazejewicz
left a comment
There was a problem hiding this comment.
optionally, consider my comment on regular user syntax, thx!
|
Ready to merge |
|
@peterblazejewicz Oh and just to elaborate since this is the second review I'm not incorporating: in any other repo I wouldn't have a problem doing these since a particular style doesn't concern me much. But PRs here take weeks to get a review. I was afraid that a push would prolong this another 3 weeks and this would interrupt other scheduled work. |
please feel free to reference me directly for that kind of changes you're doing, thx! |
Currently instances in ref callbacks are allowed to be nullable due to our bivariance hack. However, during runtime these instances can be null: https://codesandbox.io/s/refs-are-nullable-m44vxx
We'll likely remove the bivariance hack to catch these issues in the future. In the meantime, existing packages and tests should guard against nullable instances regardless.
The issue was first reported in #58464