Skip to content

Commit

Permalink
chore: refactor client code for cleanliness (microsoft#5401)
Browse files Browse the repository at this point in the history
* fix some front-end type-safety things

* fix some front-end type-safety things

* various client-side type cleanups

* Update luUtil.ts

* remove another reduce

* change various && checks to ?.

* Update settings.ts

* rearrange and fix tests

* fix some front-end type-safety things

* various client-side type cleanups

* Update luUtil.ts

* remove another reduce

* change various && checks to ?.

* Update settings.ts

* rearrange and fix tests

* Update project.test.tsx

* Update project.test.tsx

* fix typo that was making tests fail

* improve typechecks on some tests

* add more type declarations to tests

* more typechecking

* fix some front-end type-safety things

* various client-side type cleanups

* Update luUtil.ts

* remove another reduce

* change various && checks to ?.

* Update settings.ts

* rearrange and fix tests

* Update project.test.tsx

* various client-side type cleanups

* change various && checks to ?.

* rearrange and fix tests

* Update project.test.tsx

* fix typo that was making tests fail

* improve typechecks on some tests

* add more type declarations to tests

* more typechecking

* fix typing on unit tests

* Update trigger.test.tsx

* fixes from CR

* Update DesignPage.tsx

* fixes from CR
  • Loading branch information
beyackle authored Dec 31, 2020
1 parent 32a4899 commit c4893c7
Show file tree
Hide file tree
Showing 65 changed files with 528 additions and 454 deletions.
5 changes: 3 additions & 2 deletions Composer/packages/client/__tests__/shell/lgApi.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { renderHook } from '@botframework-composer/test-utils/lib/hooks';
import { renderHook, HookResult } from '@botframework-composer/test-utils/lib/hooks';
import * as React from 'react';
import { RecoilRoot } from 'recoil';

Expand Down Expand Up @@ -30,7 +30,8 @@ const state = {
// const resolvers = { lgFileResolver: jest.fn((id) => state.lgFiles.find((file) => file.id === id)) };

describe('use lgApi hooks', () => {
let removeLgTemplatesMock, initRecoilState, copyLgTemplateMock, updateLgTemplateMock, removeLgTemplateMock, result;
let removeLgTemplatesMock, initRecoilState, copyLgTemplateMock, updateLgTemplateMock, removeLgTemplateMock;
let result: HookResult<any>;

beforeEach(() => {
updateLgTemplateMock = jest.fn();
Expand Down
11 changes: 8 additions & 3 deletions Composer/packages/client/__tests__/utils/fileUtil.test.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import formatMessage from 'format-message';

import { getExtension, getBaseName, upperCaseName, loadLocale } from '../../src/utils/fileUtil';
import { getExtension, getBaseName, upperCaseName, loadLocale, getUniqueName } from '../../src/utils/fileUtil';
import httpClient from '../../src/utils/httpUtil';

jest.mock('../../src/utils/httpUtil');
Expand Down Expand Up @@ -68,3 +66,10 @@ describe('loadLocale', () => {
});
});
});

describe('File utils', () => {
it('should get a unique name', () => {
const uniqueName = getUniqueName(['test', 'test-1', 'test-2', 'test-3'], 'test');
expect(uniqueName).toBe('test-4');
});
});
3 changes: 2 additions & 1 deletion Composer/packages/client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
"test": "jest",
"lint": "eslint --quiet --ext .js,.jsx,.ts,.tsx ./src ./__tests__",
"lint:fix": "yarn lint --fix",
"typecheck": "tsc --noEmit"
"typecheck": "tsc --noEmit",
"typecheck:watch": "tsc --noEmit --watch"
},
"proxy": "http://localhost:5000",
"dependencies": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { resolveToBasePath } from '../../utils/fileUtil';
import { BASEPATH } from '../../constants';
import { NavItem } from '../NavItem';
import TelemetryClient from '../../telemetry/TelemetryClient';
import { PageLink } from '../../utils/pageLinks';

import { useLinks } from './../../utils/hooks';

Expand Down Expand Up @@ -69,7 +70,7 @@ export const SideBar = () => {

const mapNavItemTo = (relPath: string) => resolveToBasePath(BASEPATH, relPath);
const globalNavButtonText = sideBarExpand ? formatMessage('Collapse Navigation') : formatMessage('Expand Navigation');
const showTooltips = (link) => !sideBarExpand && !link.disabled;
const showTooltips = (link: PageLink) => !sideBarExpand && !link.disabled;
return (
<nav css={sideBar(sideBarExpand)}>
<div>
Expand Down
16 changes: 12 additions & 4 deletions Composer/packages/client/src/components/AppUpdater.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { ChoiceGroup } from 'office-ui-fabric-react/lib/ChoiceGroup';
import formatMessage from 'format-message';
import { useRecoilValue } from 'recoil';
import { SharedColors, NeutralColors } from '@uifabric/fluent-theme';
import { IpcRendererEvent } from 'electron';

import { AppUpdaterStatus } from '../constants';
import { appUpdateState, dispatcherState } from '../recoilModel';
Expand All @@ -28,7 +29,7 @@ const updateAvailableDismissBtn: Partial<IButtonStyles> = {
},
};

const optionIcon = (checked) => css`
const optionIcon = (checked: boolean) => css`
vertical-align: text-bottom;
font-size: 18px;
margin-right: 10px;
Expand Down Expand Up @@ -60,8 +61,15 @@ const dialogContent: Partial<IDialogContentStyles> = {

const { ipcRenderer } = window;

function SelectOption(props) {
const { checked, text, key } = props;
type SelectOptionProps = {
checked?: boolean;
text: string;
key: string;
};

function SelectOption(props: SelectOptionProps | undefined) {
if (props == null) return null;
const { checked = false, text, key } = props;
return (
<div key={key} css={optionRoot}>
<Icon css={optionIcon(checked)} iconName={checked ? 'RadioBtnOn' : 'RadioBtnOff'} />
Expand Down Expand Up @@ -110,7 +118,7 @@ export const AppUpdater: React.FC<{}> = () => {

// listen for app updater events from main process
useEffect(() => {
ipcRenderer.on('app-update', (_event, name, payload) => {
ipcRenderer.on('app-update', (_event: IpcRendererEvent, name: string, payload) => {
switch (name) {
case 'update-available':
setAppUpdateStatus(AppUpdaterStatus.UPDATE_AVAILABLE, payload.version);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { Dialog, DialogType } from 'office-ui-fabric-react/lib/Dialog';
import { FontWeights, FontSizes } from 'office-ui-fabric-react/lib/Styling';
import { DialogFooter } from 'office-ui-fabric-react/lib/Dialog';
import { PrimaryButton, DefaultButton } from 'office-ui-fabric-react/lib/Button';
import { TextField } from 'office-ui-fabric-react/lib/TextField';
import { TextField, ITextFieldProps } from 'office-ui-fabric-react/lib/TextField';
import { Link } from 'office-ui-fabric-react/lib/Link';
import { IconButton } from 'office-ui-fabric-react/lib/Button';
import { Stack } from 'office-ui-fabric-react/lib/Stack';
Expand Down Expand Up @@ -53,8 +53,8 @@ const dialogModal = {
interface FormData {
name: string;
authoringKey: string;
subscriptionKey: string;
qnaRegion: string;
subscriptionKey?: string;
qnaRegion?: string;
endpointKey: string;
authoringRegion: string;
defaultLanguage: string;
Expand All @@ -63,16 +63,16 @@ interface FormData {
authoringEndpoint: string;
}

const validate = (value: string) => {
if (!nameRegex.test(value)) {
const validate = (value?: string) => {
if (value != null && !nameRegex.test(value)) {
return formatMessage('Spaces and special characters are not allowed. Use letters, numbers, -, or _.');
}
};

// eslint-disable-next-line react/display-name
const onRenderLabel = (info) => (props) => (
const onRenderLabel = (info: string) => (props?: ITextFieldProps) => (
<Stack horizontal verticalAlign="center">
<span css={textFieldLabel}>{props.label}</span>
<span css={textFieldLabel}>{props?.label}</span>
<TooltipHost calloutProps={{ gapSpace: 0 }} content={info}>
<IconButton iconProps={{ iconName: 'Info' }} styles={{ root: { marginBottom: -3 } }} />
</TooltipHost>
Expand All @@ -99,17 +99,17 @@ export const PublishDialog: React.FC<IPublishDialogProps> = (props) => {
const formConfig: FieldConfig<FormData> = {
name: {
required: true,
validate: validate,
validate,
defaultValue: config.name || botName,
},
authoringKey: {
required: luConfigShow,
validate: validate,
validate,
defaultValue: config.authoringKey,
},
subscriptionKey: {
required: qnaConfigShow,
validate: validate,
validate,
defaultValue: config.subscriptionKey,
},
qnaRegion: {
Expand All @@ -130,7 +130,7 @@ export const PublishDialog: React.FC<IPublishDialogProps> = (props) => {
},
environment: {
required: true,
validate: validate,
validate,
defaultValue: config.environment,
},
endpoint: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import find from 'lodash/find';
import formatMessage from 'format-message';
import { PrimaryButton, DefaultButton } from 'office-ui-fabric-react/lib/Button';
import { Icon } from 'office-ui-fabric-react/lib/Icon';
import { ChoiceGroup } from 'office-ui-fabric-react/lib/ChoiceGroup';
import { ChoiceGroup, IChoiceGroupOption } from 'office-ui-fabric-react/lib/ChoiceGroup';
import { DialogFooter } from 'office-ui-fabric-react/lib/Dialog';
import { ScrollablePane, ScrollbarVisibility } from 'office-ui-fabric-react/lib/ScrollablePane';
import { Selection } from 'office-ui-fabric-react/lib/DetailsList';
Expand Down Expand Up @@ -36,7 +36,7 @@ import TelemetryClient from '../../telemetry/TelemetryClient';

// -------------------- Styles -------------------- //

const optionIcon = (checked) => css`
const optionIcon = (checked: boolean) => css`
vertical-align: text-bottom;
font-size: 18px;
margin-right: 10px;
Expand Down Expand Up @@ -65,7 +65,7 @@ export const bannerClass = mergeStyles({
marginTop: '5px',
});

const rowDetails = (disabled) => {
const rowDetails = (disabled: boolean) => {
return {
root: {
color: disabled ? NeutralColors.gray80 : NeutralColors.black,
Expand All @@ -83,7 +83,7 @@ const rowDetails = (disabled) => {
};
};

const rowTitle = (disabled) => {
const rowTitle = (disabled: boolean) => {
return {
cellTitle: {
color: disabled ? NeutralColors.gray80 : NeutralColors.black,
Expand Down Expand Up @@ -141,17 +141,19 @@ export function CreateOptions(props: CreateOptionsProps) {
});
}, []);

function SelectOption(props) {
function SelectOption(props?: { checked?: boolean; text: string; key: string }) {
if (props == null) return null;
const { checked, text, key } = props;
return (
<div key={key} css={optionRoot}>
<Icon css={optionIcon(checked)} iconName={checked ? 'CompletedSolid' : 'RadioBtnOff'} />
<Icon css={optionIcon(checked ?? false)} iconName={checked ? 'CompletedSolid' : 'RadioBtnOff'} />
<span>{text}</span>
</div>
);
}

const handleChange = (event, option) => {
const handleChange = (event, option?: IChoiceGroupOption) => {
if (option == null) return;
setOption(option.key);
if (option.key === optionKeys.createFromTemplate) {
setDisabled(false);
Expand All @@ -171,7 +173,7 @@ export function CreateOptions(props: CreateOptionsProps) {
routeToTemplate = QnABotTemplateId;
}

if (props.location && props.location.search) {
if (props.location?.search) {
routeToTemplate += props.location.search;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const CreationFlow: React.FC<CreationFlowProps> = () => {
const storage = storages[currentStorageIndex.current];
const currentStorageId = storage ? storage.id : 'default';
useEffect(() => {
if (storages && storages.length) {
if (storages?.length) {
const storageId = storage.id;
const path = storage.path;
const formattedPath = Path.normalize(path);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ export const LocationSelectContent: React.FC<LocationSelectContentProps> = (prop
const storageFileLoadingStatus = useRecoilValue(storageFileLoadingStatusState);
const currentStorageIndex = useRef(0);
const storage = storages[currentStorageIndex.current];
const isWindows = storage && storage.platform === 'win32';
const isWindows = storage?.platform === 'win32';
const onFileChosen = (item: File) => {
if (item) {
const { type, path } = item;
const storageId = storage.id;
if (type === FileTypes.FOLDER) {
onCurrentPathUpdate(path, storageId);
} else if (type === FileTypes.BOT && creationFlowStatus === CreationFlowStatus.OPEN) {
onOpen && onOpen(path, storageId);
onOpen?.(path, storageId);
}
}
};
Expand Down
6 changes: 3 additions & 3 deletions Composer/packages/client/src/components/EditableField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ const EditableField: React.FC<EditableFieldProps> = (props) => {
fieldRef.current?.focus();
};

const handleChange = (_e: any, newValue?: string) => {
const handleChange = (_e, newValue?: string) => {
if (isMultiLineText(newValue)) setMultiline(true);
updateField('value', newValue);
setHasBeenEdited(true);
Expand All @@ -170,12 +170,12 @@ const EditableField: React.FC<EditableFieldProps> = (props) => {
if (!formData.value) {
updateField('value', value);
}
onBlur && onBlur(id, formData.value);
onBlur?.(id, formData.value);
};

const handleOnFocus = () => {
setHasFocus(true);
onFocus && onFocus();
onFocus?.();
};

const cancel = () => {
Expand Down
6 changes: 3 additions & 3 deletions Composer/packages/client/src/components/ErrorBoundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import { ErrorPopup } from './ErrorPopup/ErrorPopup';

const genericErrorTitle = 'Something went wrong!';

const formatToStateError = (error: any): StateError => {
const message: string = typeof error === 'string' ? error : error?.message || error?.detail;
const summary: string = message || genericErrorTitle;
const formatToStateError = (error): StateError => {
const message: string = typeof error === 'string' ? error : error?.message ?? error?.detail;
const summary: string = message ?? genericErrorTitle;
return {
message,
summary,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ export const FieldWithCustomButton: React.FC<Props> = (props) => {
if (!localValue) {
setDisabled(true);
}
onBlur && onBlur(localValue);
onBlur?.(localValue);
},
};

Expand All @@ -196,7 +196,7 @@ export const FieldWithCustomButton: React.FC<Props> = (props) => {
value={localValue}
onChange={(e, value) => {
setLocalValue(value ?? '');
onChange && onChange(e, value);
onChange?.(e, value);
}}
/>
) : (
Expand All @@ -207,7 +207,7 @@ export const FieldWithCustomButton: React.FC<Props> = (props) => {
selectedKey={localValue}
onChange={(e, option: IDropdownOption | undefined) => {
setLocalValue((option?.key as string) ?? '');
onChange && onChange(e, option?.key);
onChange?.(e, option?.key);
}}
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export const ImportModal: React.FC<RouteComponentProps> = (props) => {
useEffect(() => {
if (modalState === 'downloadingContent') {
const importBotContent = async () => {
if (location && location.href) {
if (location?.href) {
try {
const { description, name } = importPayload;

Expand Down Expand Up @@ -198,7 +198,7 @@ export const ImportModal: React.FC<RouteComponentProps> = (props) => {
}, [modalState, importSource, importPayload]);

useEffect(() => {
if (location && location.href) {
if (location?.href) {
try {
// parse data from url and store in state
const url = new URL(location.href);
Expand Down
Loading

0 comments on commit c4893c7

Please sign in to comment.