facebook
further restrict property descriptor type definitions
I was getting an error on this code due to a loose definition of property descriptors:
function safeWrite(obj: {}, prop: string, value: any) {
let desc = Object.getOwnPropertyDescriptor(obj, prop);
if (desc && 'value' in desc && (desc.writable || desc.configurable)) {
desc.value = value;
Object.defineProperty(obj, prop, desc);
}
}
The error message was
Sketchy null check on boolean [1] which is potentially false. Perhaps you meant to check for null or undefined [1]?
(sketchy-null-bool)
xxx.js
8│
9│ function safeWrite(obj: {}, prop: string, value: any) {
10│ let desc = Object.getOwnPropertyDescriptor(obj, prop);
11│ if (desc && 'value' in desc && (desc.writable || desc.configurable)) {
12│ desc.value = value;
13│ Object.defineProperty(obj, prop, desc);
14│ }
/tmp/flow/flowlib_16ab40eb/core.js
[1] 27│ writable?: boolean;
It seems tests are failing because I haven't updated error offsets. How do I go about doing that? I can't find development documentation.
Could you provide some documentation that backs up the changes you made here?
@mrkev Are you asking for me to add documentation to the PR or are you asking for me to provide you links to the sections of the spec that define the affected functions?
For more information about the Property Descriptor specification type, see https://tc39.github.io/ecma262/#sec-property-descriptor-specification-type.
ContraPropertyDescriptor represents the property descriptor-like objects accepted by APIs which manipulate property descriptors. These objects are converted to proper property descriptors by ToPropertyDescriptor.
CoPropertyDescriptor represents objects returned by APIs which read property descriptors. These objects are more restricted as each of the property descriptor's properties will always be present.
@mrkev I've provided the information that you asked for here. Can we get this merged?
@mrkev It's been another few months and I've yet again been having difficulties with Flow due to its loose typing of property descriptors. Is there anything else I can do to get this PR merged? I believe we just need to update error offsets, but there's no documentation on how to do that.
@mrkev I run into this issue nearly every time I write code that uses Flow. The implementation work here has already been done. Is there anything I can do to get somebody to merge this? I would think the Flow team would be more concerned about incorrect descriptions of JavaScript standard library APIs.
@goodmind @thread been swamped with FB5 work. Probably won't be able to look at it for another while, sorry /:
@mrkev is there a different maintainer that can look at it? It's not a very complicated change.