Support error reporting for missing props #47
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks! If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions. |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
@gabelevi Any comments on this? |
| let details = []; | ||
| other.forEach(part => { | ||
| let partMsg = part.descr; | ||
| if (partMsg && partMsg !== 'null' && partMsg !== 'undefined') { |
There was a problem hiding this comment.
Weird - why is it checking for the strings 'null' and 'undefined'?
There was a problem hiding this comment.
The error reporter in Nuclide doesn't have this check so I think it's safe to remove it.
| diags[message.file] = []; | ||
| } | ||
|
|
||
| message.msg = description.reduce((full, descr, i, arr) => { |
There was a problem hiding this comment.
It seems like description can be an empty array, which means this might throw an exception. Maybe only call reduce if arr description is non-empty?
|
Given this a shot - looks good, should be good to merge IMO
Ideally we need support in VS Code for something like microsoft/vscode#10271 - but we could live with microsoft/vscode#1927 |
|
Quick note: a similar PR was rejected in Nuclide. Here's the reasoning: facebookarchive/nuclide#845 (comment) |
ghost
mentioned this pull request
|
Will this be merged or is it dead? |
|
I'm assuming it's dead |
In general, support error reporting in all ranges when a message points to more than one file.