-
Notifications
You must be signed in to change notification settings - Fork 49.6k
[Flight] Compute better I/O description for exotic types #34650
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Flight] Compute better I/O description for exotic types #34650
Conversation
Comparing: 319a786...9d47086 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
export function getIODescription(value: mixed): string { | ||
if (!__DEV__) { | ||
return ''; | ||
} | ||
try { | ||
switch (typeof value) { | ||
case 'object': | ||
// Test the object for a bunch of common property names that are useful identifiers. | ||
// While we only have the return value here, it should ideally be a name that | ||
// describes the arguments requested. | ||
if (value === null) { | ||
return ''; | ||
} else if (value instanceof Error) { | ||
// eslint-disable-next-line react-internal/safe-string-coercion | ||
return String(value.message); | ||
} else if (typeof value.url === 'string') { | ||
return value.url; | ||
} else if (typeof value.href === 'string') { | ||
return value.href; | ||
} else if (typeof value.src === 'string') { | ||
return value.src; | ||
} else if (typeof value.currentSrc === 'string') { | ||
return value.currentSrc; | ||
} else if (typeof value.command === 'string') { | ||
return value.command; | ||
} else if ( | ||
typeof value.request === 'object' && | ||
value.request !== null && | ||
typeof value.request.url === 'string' | ||
) { | ||
return value.request.url; | ||
} else if ( | ||
typeof value.response === 'object' && | ||
value.response !== null && | ||
typeof value.response.url === 'string' | ||
) { | ||
return value.response.url; | ||
} else if ( | ||
typeof value.id === 'string' || | ||
typeof value.id === 'number' || | ||
typeof value.id === 'bigint' | ||
) { | ||
// eslint-disable-next-line react-internal/safe-string-coercion | ||
return String(value.id); | ||
} else if (typeof value.name === 'string') { | ||
return value.name; | ||
} else { | ||
const str = value.toString(); | ||
if (str.startWith('[object ') || str.length < 5 || str.length > 500) { | ||
if ( | ||
str.startsWith('[object ') || | ||
str.length < 5 || | ||
str.length > 500 | ||
) { | ||
// This is probably not a useful description. | ||
return ''; | ||
} | ||
return str; | ||
} | ||
case 'string': | ||
if (value.length < 5 || value.length > 500) { | ||
return ''; | ||
} | ||
return value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole thing is inside a try/catch so how could you have gotten a crash? Wouldn't it just be empty string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Also the new GitHub UI is very buggy. This is not the line I commented on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't see a crash but me stepping through with pausing on caught exceptions. Didn't see the try-catch higher up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated title and PR desc
Could also affect DevTools. Bumped into this with "TypeError: str.startWith is not a function". Stricter types revealed some more potential misses.