Skip to content
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

Michael/perf 1699 #270

Merged
merged 4 commits into from
Nov 27, 2024
Merged

Michael/perf 1699 #270

merged 4 commits into from
Nov 27, 2024

Conversation

blonde-mike
Copy link
Collaborator

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.

@summersjc
Copy link
Collaborator

Already approved

@blonde-mike blonde-mike merged commit 319f9d5 into master Nov 27, 2024
3 checks passed
@blonde-mike blonde-mike deleted the michael/PERF-1699 branch November 27, 2024 20:50
@tkmcmaster
Copy link
Collaborator

@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.

Copy link
Collaborator

@tkmcmaster tkmcmaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 })

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably should actually look at the status code. There's other errors like CORS errors that are not being displayed properly

tkmcmaster added a commit that referenced this pull request Dec 6, 2024
tkmcmaster added a commit that referenced this pull request Dec 6, 2024
* 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 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants