Skip to content

Commit

Permalink
fix: select only the required values from state and useEffect to get …
Browse files Browse the repository at this point in the history
…description error message (#3395)

* Update dependencies and workspace

* Select only the required values from the state

Having the selector returning the entire state and then using spread operators to get the values we want will cause components to rerender. This will impact performance because if a component has the entire state and a part of the state it does not use is updated, the component has to rerender. To avoid this, only get what you need from the state for a component.

* Ensure that AutoComplete finishes rendering before calling ValidationProvider

ValidationProvider is updating when AutoComplete tries to call it while rendering.
  • Loading branch information
musale authored Oct 30, 2024
1 parent b99029e commit 9141255
Show file tree
Hide file tree
Showing 25 changed files with 105 additions and 58 deletions.
3 changes: 3 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,7 @@
"*.tsx": "$(capture).ts, $(capture).d.ts, $(capture).spec.tsx, $(capture).styles.ts",
"package.json": "package-lock.json, .npmrc"
},
"cSpell.words": [
"fluentui"
],
}
33 changes: 22 additions & 11 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"eslint-config-react-app": "7.0.1",
"eslint-plugin-react": "7.33.2",
"eslint-webpack-plugin": "4.1.0",
"express": "4.21.0",
"express": "^4.21.1",
"expvariantassignmentsdk": "file:packages/expvariantassignmentsdk-1.0.0.tgz",
"file-loader": "6.2.0",
"fork-ts-checker-webpack-plugin": "9.0.2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ interface ValidationProviderProps {
}

export const ValidationProvider = ({ children }: ValidationProviderProps) => {
const { resources } = useAppSelector((state) => state);
const resources = useAppSelector((state) => state.resources);
const base = Object.keys(resources.data).length > 0 ?
resources.data[GRAPH_API_VERSIONS[0]].children! : [];

Expand Down
4 changes: 2 additions & 2 deletions src/app/views/app-sections/StatusMessages.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import MessageDisplay from '../common/message-display/MessageDisplay';

const StatusMessages = () => {
const dispatch = useAppDispatch();
const { queryRunnerStatus, sampleQuery } =
useAppSelector((state) => state);
const queryRunnerStatus = useAppSelector((state)=> state.queryRunnerStatus);
const sampleQuery = useAppSelector((state)=> state.sampleQuery);

function setQuery(link: string) {
const query: IQuery = { ...sampleQuery };
Expand Down
3 changes: 1 addition & 2 deletions src/app/views/app-sections/TermsOfUseMessage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ import { appStyles } from '../App.styles';

const StyledTermsOfUseMessage = () => {

const { termsOfUse } =
useAppSelector((state) => state);
const termsOfUse = useAppSelector((state) => state.termsOfUse);

const dispatch = useAppDispatch();
if (termsOfUse) {
Expand Down
2 changes: 1 addition & 1 deletion src/app/views/authentication/Authentication.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { translateMessage } from '../../utils/translate-messages';
const Authentication = (props: any) => {
const dispatch: AppDispatch = useAppDispatch();
const [loginInProgress, setLoginInProgress] = useState(false);
const { auth: { authToken } } = useAppSelector((state) => state);
const authToken = useAppSelector((state) => state.auth.authToken);
const tokenPresent = !!authToken.token;
const logoutInProgress = !!authToken.pending;

Expand Down
3 changes: 2 additions & 1 deletion src/app/views/main-header/FeedbackButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import { useAppSelector } from '../../../store';

export const FeedbackButton = () => {
const [enableSurvey, setEnableSurvey] = useState(false);
const { profile: { user } } = useAppSelector((state) => state);
const user = useAppSelector((state) => state.profile.user)

const currentTheme = getTheme();
const feedbackIcon : IIconProps = {
iconName : 'Feedback'
Expand Down
3 changes: 2 additions & 1 deletion src/app/views/main-header/Help.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import { mainHeaderStyles } from './MainHeader.styles';
import { useAppSelector } from '../../../store';

export const Help = () => {
const { auth: { authToken } } = useAppSelector((state) => state);
const auth = useAppSelector((state)=> state.auth)
const authToken = auth.authToken;
const authenticated = authToken.token;
const [items, setItems] = useState([]);
const currentTheme = getTheme();
Expand Down
7 changes: 4 additions & 3 deletions src/app/views/main-header/MainHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ registerIcons({
}
});
export const MainHeader: React.FunctionComponent<MainHeaderProps> = (props: MainHeaderProps) => {
const { profile: { user }, graphExplorerMode, sidebarProperties } = useAppSelector(
(state) => state
);
const profile = useAppSelector((state)=> state.profile)
const user = profile.user;
const graphExplorerMode = useAppSelector((state)=> state.graphExplorerMode)
const sidebarProperties = useAppSelector((state)=> state.sidebarProperties)

const mobileScreen = !!sidebarProperties.mobileScreen;
const showSidebar = !!sidebarProperties.showSidebar;
Expand Down
3 changes: 2 additions & 1 deletion src/app/views/main-header/settings/Settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import { translateMessage } from '../../../utils/translate-messages';
import { mainHeaderStyles } from '../MainHeader.styles';

export const Settings: React.FunctionComponent<ISettingsProps> = () => {
const { auth: { authToken } } = useAppSelector((state) => state);
const auth = useAppSelector((state)=> state.auth)
const authToken = auth.authToken;
const authenticated = authToken.token;
const [items, setItems] = useState([]);
const currentTheme = getTheme();
Expand Down
4 changes: 3 additions & 1 deletion src/app/views/query-response/QueryResponse.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ const QueryResponse = () => {
const dispatch = useAppDispatch();
const [showModal, setShowModal] = useState(false);
const [responseHeight, setResponseHeight] = useState('610px');
const { sampleQuery, dimensions, snippets } = useAppSelector((state) => state);
const sampleQuery = useAppSelector((state)=> state.sampleQuery);
const dimensions = useAppSelector((state)=> state.dimensions);
const snippets = useAppSelector((state)=> state.snippets);
const [currentTab, setCurrentTab] = useState<string>('response-preview');
const currentTheme: ITheme = getTheme();
const { modalStyles, modalPivotStyles } = queryResponseStyles(currentTheme);
Expand Down
6 changes: 3 additions & 3 deletions src/app/views/query-response/pivot-items/pivot-items.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ import {
} from '../../common/lazy-loader/component-registry';

export const GetPivotItems = () => {

const { graphExplorerMode: mode, sampleQuery,
graphResponse: { response: { body } } } = useAppSelector((state) => state);
const mode = useAppSelector((state)=> state.graphExplorerMode);
const sampleQuery= useAppSelector((state)=> state.sampleQuery);
const body = useAppSelector((state)=> state.graphResponse.response.body);

const currentTheme: ITheme = getTheme();
const dotStyle = queryResponseStyles(currentTheme).dot;
Expand Down
5 changes: 3 additions & 2 deletions src/app/views/query-response/queryResponse.styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ import { useAppSelector } from '../../../store';
import { convertVhToPx, getResponseHeight } from '../common/dimensions/dimensions-adjustment';

export const queryResponseStyles = (theme: ITheme) => {
const { dimensions: { response }, responseAreaExpanded} =
useAppSelector((state) => state);
const response = useAppSelector((state)=> state.dimensions.response);
const responseAreaExpanded = useAppSelector((state)=> state.responseAreaExpanded);


const height = convertVhToPx(getResponseHeight( response.height, responseAreaExpanded), 220);

Expand Down
6 changes: 4 additions & 2 deletions src/app/views/query-response/response/Response.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ import ResponseDisplay from './ResponseDisplay';
import { ResponseMessages } from './ResponseMessages';

const Response = () => {
const { dimensions: { response }, graphResponse: { response: { body, headers } }, responseAreaExpanded } =
useAppSelector((state) => state);
const response = useAppSelector((state) => state.dimensions.response);
const body = useAppSelector((state) => state.graphResponse.response.body);
const headers = useAppSelector((state) => state.graphResponse.response.headers);
const responseAreaExpanded = useAppSelector((state) => state.responseAreaExpanded);

const defaultHeight = convertVhToPx(getResponseHeight(response.height, responseAreaExpanded), 220);
const monacoHeight = getResponseEditorHeight(150);
Expand Down
8 changes: 5 additions & 3 deletions src/app/views/query-response/response/ResponseMessages.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,11 @@ function getOdataLinkFromResponseBody(responseBody: any): ODataLink | null {
export const ResponseMessages = () => {
const dispatch = useAppDispatch();
const messageBars = [];

const { graphResponse: { response: { body, headers } }, sampleQuery, auth: { authToken }, graphExplorerMode
} = useAppSelector((state) => state);
const body = useAppSelector((state)=> state.graphResponse.response.body);
const headers = useAppSelector((state)=> state.graphResponse.response.headers);
const sampleQuery = useAppSelector((state)=> state.sampleQuery);
const authToken= useAppSelector((state)=> state.auth.authToken);
const graphExplorerMode = useAppSelector((state)=> state.graphExplorerMode);
const [displayMessage, setDisplayMessage] = useState(true);

const tokenPresent = !!authToken.token;
Expand Down
2 changes: 1 addition & 1 deletion src/app/views/query-runner/QueryRunner.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import Request from './request/Request';

const QueryRunner = (props: any) => {
const dispatch = useAppDispatch();
const { sampleQuery } = useAppSelector((state) => state);
const sampleQuery = useAppSelector((state) => state.sampleQuery);

const [sampleBody, setSampleBody] = useState('');

Expand Down
7 changes: 4 additions & 3 deletions src/app/views/query-runner/query-input/QueryInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,11 @@ const QueryInput = (props: IQueryInputProps) => {
})
});

const { sampleQuery, auth: { authToken },
graphResponse: { isLoadingData },
sidebarProperties } = useAppSelector((state) => state);
const sampleQuery = useAppSelector((state) => state.sampleQuery);
const authToken = useAppSelector((state) => state.auth.authToken);
const authenticated = !!authToken.token;
const isLoadingData = useAppSelector((state) => state.graphResponse.isLoadingData);
const sidebarProperties = useAppSelector((state) => state.sidebarProperties);
const { mobileScreen } = sidebarProperties;

const showError = !shouldRunQuery({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ const AutoComplete = (props: IAutoCompleteProps) => {

let element: HTMLDivElement | null | undefined = null;

const { sampleQuery, autoComplete: { data: autoCompleteOptions, pending: autoCompletePending } } = useAppSelector(
(state) => state
);
const sampleQuery = useAppSelector((state)=> state.sampleQuery);
const autoCompleteOptions = useAppSelector((state)=> state.autoComplete.data);
const autoCompletePending = useAppSelector((state)=> state.autoComplete.pending);

const previousQuery = usePrevious(sampleQuery.sampleUrl);
const [isMultiline, setIsMultiline] = useState<boolean>(false);
Expand Down Expand Up @@ -290,6 +290,14 @@ const AutoComplete = (props: IAutoCompleteProps) => {
validation.validate(queryUrl);
return validation.error;
}
const [descriptionError, setDescriptionError] = useState('');

useEffect(()=>{
const errorMessage = getErrorMessage();
if (errorMessage) {
setDescriptionError(errorMessage)
}
}, [getErrorMessage])

return (
<div onBlur={closeSuggestionDialog}>
Expand All @@ -310,7 +318,7 @@ const AutoComplete = (props: IAutoCompleteProps) => {
ariaLabel={translateMessage('Query Sample Input')}
role='textbox'
onRenderDescription={handleRenderDescription}
description={getErrorMessage()}
description={descriptionError}
/>
</div>
{shouldShowSuggestions && queryUrl && suggestions.length > 0 &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import DocumentationService from './documentation';
import { styles } from './suffix.styles';

const SuffixRenderer = () => {
const { sampleQuery, samples, resources } = useAppSelector(
(state) => state
);
const sampleQuery = useAppSelector((state)=> state.sampleQuery);
const samples = useAppSelector((state)=> state.samples);
const resources = useAppSelector((state)=> state.resources);

const buttonId = getId('callout-button');
const calloutProps = { gapSpace: 0 };
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useRef, useEffect } from 'react';

const usePrevious = (value: string) => {
const reference = useRef<any>(null);
const reference = useRef<unknown>(null);
useEffect(() => {
reference.current = value;
});
Expand Down
18 changes: 13 additions & 5 deletions src/app/views/query-runner/request/Request.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,27 @@ import { convertPxToVh, convertVhToPx } from '../../common/dimensions/dimensions
import { Auth, Permissions, RequestHeaders } from '../../common/lazy-loader/component-registry';
import { RequestBody } from './body';
import './request.scss';
import { IQuery } from '../../../../types/query-runner';

const Request = (props: any) => {
interface IRequestProps {
handleOnEditorChange: ()=> void
sampleQuery: IQuery
}

const Request = (props: IRequestProps) => {
const dispatch = useAppDispatch();
const [selectedPivot, setSelectedPivot] = useState('request-body');
const { graphExplorerMode: mode, dimensions, sidebarProperties } = useAppSelector((state) => state);
const mode = useAppSelector((state)=> state.graphExplorerMode);
const dimensions= useAppSelector((state)=> state.dimensions);
const sidebarProperties = useAppSelector((state)=> state.sidebarProperties);
const pivot = selectedPivot.replace('.$', '');
const minHeight = 60;
const maxHeight = 800;

const {
handleOnEditorChange
}: any = props;
handleOnEditorChange,
sampleQuery
}: IRequestProps = props;

useEffect(() => {
if(sidebarProperties && sidebarProperties.mobileScreen){
Expand Down Expand Up @@ -129,7 +138,6 @@ const Request = (props: any) => {
const onPivotItemClick = (item?: PivotItem) => {
if (!item) { return; }
const tabKey = item.props.itemKey;
const { sampleQuery }: any = props;
if (tabKey) {
telemetry.trackTabClickEvent(tabKey, sampleQuery);
}
Expand Down
11 changes: 8 additions & 3 deletions src/app/views/query-runner/request/body/RequestBody.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,18 @@ import { useAppSelector } from '../../../../../store';
import { Monaco } from '../../../common';
import { convertVhToPx } from '../../../common/dimensions/dimensions-adjustment';

const RequestBody = ({ handleOnEditorChange }: any) => {
const { dimensions: { request: { height } }, sampleQuery } = useAppSelector((state) => state);
interface IRequestBodyProps {
handleOnEditorChange: (v: string | undefined)=> void;
}

const RequestBody = ({ handleOnEditorChange }: IRequestBodyProps) => {
const height = useAppSelector((state)=> state.dimensions.request.height);
const sampleBody = useAppSelector((state)=> state.sampleQuery.sampleBody);

return (
<FocusZone>
<Monaco
body={sampleQuery.sampleBody}
body={sampleBody}
height={convertVhToPx(height, 60)}
onChange={(value) => handleOnEditorChange(value)} />
</FocusZone>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export default function FeedbackForm({ activated, onDismissSurvey, onDisableSurv
const [officeBrowserFeedback, setOfficeBrowserFeedback] = useState<any>(undefined);
const currentTheme = getTheme();
const { NODE_ENV } = process.env;
const { profile: { user } } = useAppSelector((state) => state);
const user = useAppSelector((state) => state.profile.user);

function surveyActivated(launcher: any, surveyItem: any) {
return surveyItem;
Expand Down
Loading

0 comments on commit 9141255

Please sign in to comment.