Michael/perf 1699#270
Conversation
|
Already approved |
|
@blonde-mike Testing this locally it doesn't seem to be working. I'm getting CORS header errors making the requests. on a few test urls. |
tkmcmaster
left a comment
tkmcmaster
left a comment
There was a problem hiding this comment.
@blonde-mike I added some comments to the PR.
|
|
||
| interface RequestDetailsTabsProps extends HeadersViewProps, ResponseViewProps {} | ||
|
|
||
| function RequestDetailsTabs ({ id, headersList, removeHeader, changeHeader, addHeader, responseCode, response }: RequestDetailsTabsProps): JSX.Element { |
There was a problem hiding this comment.
When creating sub elements, it's a bit cleaner to put them in their own file and create a separate storybook for them.
| addHeader: (headerType?: HeaderType) => void; | ||
| } | ||
|
|
||
| function HeadersView ({ id, headersList, removeHeader, changeHeader, addHeader }: HeadersViewProps): JSX.Element { |
There was a problem hiding this comment.
When creating sub elements, it's a bit cleaner to put them in their own file and create a separate storybook for them.
| }; | ||
| try { | ||
| const response = await axios(request); | ||
| setLastResponseCode(response.status + " " + response.statusText); |
There was a problem hiding this comment.
If you're using multiple useState hooks together it's more efficient to use a single state object and set them at the same time. Ex. setLastResponse({ data: response.data, status: response.status + " " + response.statusText })
There was a problem hiding this comment.
Here's a guide for when to merge vs. keeping them separate.
https://legacy.reactjs.org/docs/hooks-faq.html#should-i-use-one-or-many-state-variables
| setLastResponse(response.data); | ||
| return response.data; | ||
| } catch (error) { | ||
| throw error; |
There was a problem hiding this comment.
Throwing the error are we displaying the error to the user somehow? Locally I'm having to look at the console to see the CORS errors I was getting.
There was a problem hiding this comment.
(error as AxiosError).isAxiosError will tell you if it's an AxiosError type.
| acc[header.name] = header.value; | ||
| return acc; | ||
| }, {} as Record<string, string>), | ||
| validateStatus: () => true |
There was a problem hiding this comment.
This probably should actually look at the status code. There's other errors like CORS errors that are not being displayed properly
* Fix multiple test results bug (#269) * Fixed the bug with multiple results files and switching between - The null pointer throws on redraw. All the calculations and freeing of memory must be done within a setState block while the data is static * Updated eslint to include storybook * Updated eslint to include storybook * Fixed eslint files * Michael/perf 1699 (#270) * Updated Request tool to get it online * Removed unused component * Updates to remove lint errors * PewPew improvements (#271) * PewPew improvements * Updated scripts for running locally * Remove har to yaml from controller (#272) * Removed the yamlwriter and all its components from the controller * Changed the yamlwriter link in the layout to go to the guide yaml writer * Removed Non Prod message for login * Removed unneeded vars --------- Co-authored-by: blonde-mike <macrowther7+git@gmail.com>
* Updated Request tool to get it online * Removed unused component * Updates to remove lint errors
3 participants
Cleaned Edit URL modal and got endpoint Test button online. Does not yet handle request body or url params, but can make a basic GET request and display the results.