-
Notifications
You must be signed in to change notification settings - Fork 47.1k
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
DevTools error boundary: Search for pre-existing GH issues #21279
Conversation
Comparing: 96d00b9...d61818f 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) |
packages/react-devtools-shared/src/devtools/views/ErrorBoundary.js
Outdated
Show resolved
Hide resolved
1efcd60
to
429b542
Compare
const filters = [ | ||
// Unfortunately "repo" and "org" filters don't work | ||
// Hopefully the label filter will be sufficient. | ||
'in:title', | ||
'is:issue', | ||
'is:open', | ||
'is:public', | ||
'label:"Component: Developer Tools"', | ||
]; |
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.
Are you sure you're using the filters correctly? This works A-OK:
https://api.github.com/search/issues?q=repo:facebook/react+is:issue+is:open+is:public+in:title+Error
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.
Are you sure you're using the filters correctly?
Yes, it works when used in the URL but it doesn't work when passed to the JS API. When I add either repo
or org
filters to the query it fails with:
The listed users and repositories cannot be searched either because the resources do not exist or you do not have permission to view them.
Please let me know if you can figure out a way to make this work. I'd be happy to tweak the search.
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 just ran this through Octokit and it provided the right results:
await octokit.rest.search.issuesAndPullRequests({ q: 'Error repo:facebook/react is:issue is:open' })
{
"total_count": 191,
"incomplete_results": false,
"items": Array(30)
}
await octokit.rest.search.issuesAndPullRequests({ q: 'Error org:facebook is:issue is:open' })
{
"total_count": 5269,
"incomplete_results": false,
"items": Array(30)
}
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.
That's so bizarre. Literally what I tried doing yesterday and it wasn't working.
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.
Previously the error boundary UI in DevTools showed a link to report the error on GitHub. This was helpful but resulted in a lot of duplicate issues. This commit adds an intermediate step of searching GitHub issues (using the public API) to find a pre-existing match and linking to it instead if one is found. Hopefully this will encourage people to add info to existing issues rather than report duplicates.
429b542
to
b9b6de4
Compare
I'm going to move forward with this so I can test it within Facebook. Happy to follow up with any PR suggestions or improvements after the fact. :) |
Previously the error boundary UI in DevTools showed a link to report the error on GitHub. This was helpful but resulted in a lot of duplicate issues. This commit adds an intermediate step of searching GitHub issues (using the public API) to find a pre-existing match and linking to it instead if one is found. Hopefully this will encourage people to add info to existing issues rather than report duplicates.
Note that I'm using the rate-limited (10 requests per minute) no-auth-token API and so a fallback to the previous behavior is in place for both timeouts and errors.
No matching issues
If DevTools can't find a likely match, it will show the "report" link like before (with an added prompt to please include repro steps)
Matching issue
If DevTools finds a likely match, it links to that issue instead (to hopefully encourage people to add additional context).