From cd5120fa1894844ea2919d2cc3da1efca566eb2c Mon Sep 17 00:00:00 2001 From: Desmond Howard Date: Fri, 30 Jun 2023 12:19:11 -0700 Subject: [PATCH] Copilot Chat: Fix ESLint violations + enable linter during build (#1710) ### Motivation and Context fix the ESLint violations in CopilotChat and enable the linter to run during `yarn start`/`yarn build`. **lint errors going forward will fail the build** and will need to be corrected before a PR can merge. we have already configured the linter to auto-fix on save in VSCode and for other IDEs the `yarn lint:fix` script can be used. ### Description - `.node-pr.yml`: removed unneeded directory filter since `samples/apps` no longer contains a `yarn.lock`. - `.vscode/settings.json`: updated to point to the `eslintConfig` in the `package.json` rather than the shared JS file. this is required so that the plugins are resolved correctly by the extension. - `.eslintrc.js`: - removed deprecated `@typescript-eslint` rules. - removed plugins already resolved from create-react-app. - removed `standard-with-typescript` extended config in favor of more widely used configs. - `webapp/package.json`: - pinned TS version to `5.0.4`, the latest version below `5.1.0`. this is a dependency requirement of `@typescript-eslint` and otherwise the linter will not work. - re-added `eslintConfig` that now extends from the `../../eslintrc.js`. this is required so that the plugins are resolved correctly by ESLint. - `samples/apps/package.json`: - deleted since there are now no shared dependencies that need to be downloaded. - similarly, the `yarn.lock` file was removed as well. - all of the plugins/configs used in `.eslintrc.js` are already included in the [create-react-app config](https://github.com/facebook/create-react-app/blob/main/packages/eslint-config-react-app/index.js) which we extend. - **all other files**: fixed **379** errors and **42** warnings. ### Preview #### During build image #### In VSCode w/ ESLint extension installed image ### Contribution Checklist - [X] The code builds clean without any errors or warnings - [x] The PR follows SK Contribution Guidelines (https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md) - [X] The code follows the .NET coding conventions (https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions) verified with `dotnet format` - [X] All unit tests pass, and I have added new tests where possible - [X] I didn't break anyone :smile: - [X] Bugbash to double check nothing is broken (not yet but before merging) --- .github/workflows/node-pr.yml | 6 +- .vscode/settings.json | 3 +- samples/apps/.eslintrc.js | 45 +- .../apps/copilot-chat-app/webapp/.env.local | 2 - .../apps/copilot-chat-app/webapp/package.json | 10 +- .../apps/copilot-chat-app/webapp/src/App.tsx | 73 +- .../webapp/src/components/FileUploader.tsx | 4 +- .../webapp/src/components/chat/ChatInput.tsx | 76 +- .../src/components/chat/ChatResourceList.tsx | 8 +- .../webapp/src/components/chat/ChatRoom.tsx | 4 +- .../webapp/src/components/chat/ChatWindow.tsx | 41 +- .../src/components/chat/ShareBotMenu.tsx | 37 +- .../chat/chat-history/ChatHistoryItem.tsx | 4 +- .../components/chat/chat-list/ChatList.tsx | 11 +- .../components/chat/chat-list/NewBotMenu.tsx | 29 +- .../InvitationCreateDialog.tsx | 7 +- .../InvitationJoinDialog.tsx | 66 +- .../chat/plan-viewer/PlanStepCard.tsx | 22 +- .../chat/plan-viewer/PlanStepInput.tsx | 2 +- .../chat/plan-viewer/PlanViewer.tsx | 43 +- .../chat/prompt-details/PromptDetails.tsx | 2 +- .../chat/typing-indicator/TypingIndicator.tsx | 2 +- .../TypingIndicatorRenderer.tsx | 10 +- .../src/components/header/UserSettings.tsx | 4 +- .../open-api-plugins/PluginConnector.tsx | 84 +- .../open-api-plugins/PluginGallery.tsx | 2 +- .../webapp/src/components/utils/TextUtils.ts | 4 +- .../src/components/views/BackendProbe.tsx | 25 +- .../webapp/src/components/views/Login.tsx | 4 +- .../views/MissingEnvVariablesError.tsx | 3 +- .../copilot-chat-app/webapp/src/index.tsx | 9 +- .../webapp/src/libs/auth/AuthHelper.ts | 9 +- .../webapp/src/libs/auth/TokenHelper.ts | 39 +- .../webapp/src/libs/models/ChatParticipant.ts | 2 +- .../webapp/src/libs/models/ChatSession.ts | 4 +- .../webapp/src/libs/models/Plan.ts | 4 +- .../libs/semantic-kernel/model/AskResult.ts | 4 +- .../libs/semantic-kernel/model/KeyConfig.ts | 24 +- .../webapp/src/libs/services/BaseService.ts | 34 +- .../webapp/src/libs/services/BotService.ts | 12 +- .../webapp/src/libs/services/ChatService.ts | 69 +- .../webapp/src/libs/services/SpeechService.ts | 14 +- .../webapp/src/libs/useChat.ts | 125 +- .../webapp/src/libs/useFile.ts | 6 +- .../webapp/src/libs/utils/PlanUtils.ts | 2 +- .../webapp/src/redux/app/rootReducer.ts | 7 +- .../webapp/src/redux/app/store.ts | 26 +- .../webapp/src/redux/features/app/AppState.ts | 11 +- .../webapp/src/redux/features/app/appSlice.ts | 23 +- .../redux/features/conversations/ChatState.ts | 2 +- .../conversations/ConversationsState.ts | 8 +- .../conversations/conversationsSlice.ts | 65 +- .../message-relay/signalRMiddleware.ts | 94 +- .../redux/features/plugins/PluginsState.ts | 42 +- .../redux/features/plugins/pluginsSlice.ts | 6 +- .../apps/copilot-chat-app/webapp/yarn.lock | 4192 ++++++++--------- samples/apps/package.json | 16 - samples/apps/yarn.lock | 3339 ------------- 58 files changed, 2660 insertions(+), 6161 deletions(-) delete mode 100644 samples/apps/copilot-chat-app/webapp/.env.local delete mode 100644 samples/apps/package.json delete mode 100644 samples/apps/yarn.lock diff --git a/.github/workflows/node-pr.yml b/.github/workflows/node-pr.yml index 32e966752d..1bdabff5ab 100644 --- a/.github/workflows/node-pr.yml +++ b/.github/workflows/node-pr.yml @@ -33,10 +33,8 @@ jobs: yarndirs=() for lockfile in samples/apps/**/yarn.lock; do # loop over all yarn.lock files dir=$(dirname "$lockfile") # get the directory of the lock file - if [[ "$dir" != "samples/apps" ]]; then # exclude samples/apps directory - echo "Found yarn project in $dir" - yarndirs+=("$dir") # add the directory to the yarndirs array - fi + echo "Found yarn project in $dir" + yarndirs+=("$dir") # add the directory to the yarndirs array done echo "All yarn projects found: '${yarndirs[*]}'" diff --git a/.vscode/settings.json b/.vscode/settings.json index 6fcbd719ff..f07c5679f7 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -57,9 +57,8 @@ } ], "eslint.options": { - "overrideConfigFile": ".eslintrc.js" + "overrideConfigFile": "./package.json" }, - "eslint.packageManager": "yarn", "files.associations": { "*.json": "jsonc" }, diff --git a/samples/apps/.eslintrc.js b/samples/apps/.eslintrc.js index 7b4da79674..3755ebea6f 100644 --- a/samples/apps/.eslintrc.js +++ b/samples/apps/.eslintrc.js @@ -1,46 +1,33 @@ module.exports = { env: { - browser: true, es2021: true, }, - extends: ['plugin:react/recommended', 'standard-with-typescript'], - ignorePatterns: ['build', '.*.js', '*.config.js', 'node_modules'], - overrides: [], + extends: [ + 'eslint:recommended', + 'plugin:react/recommended', + 'plugin:react-hooks/recommended', + 'plugin:@typescript-eslint/recommended', + 'plugin:@typescript-eslint/recommended-requiring-type-checking', + 'plugin:@typescript-eslint/strict', + ], + ignorePatterns: ['build', '.*.js', 'node_modules'], parserOptions: { project: './tsconfig.json', ecmaVersion: 'latest', sourceType: 'module', }, - plugins: ['react', '@typescript-eslint', 'import', 'react-hooks', 'react-security'], rules: { - '@typescript-eslint/brace-style': ['off'], - '@typescript-eslint/space-before-function-paren': [ - 'error', - { anonymous: 'always', named: 'never', asyncArrow: 'always' }, - ], - '@typescript-eslint/semi': ['error', 'always'], + '@typescript-eslint/array-type': ['error', { default: 'array-simple' }], '@typescript-eslint/triple-slash-reference': ['error', { types: 'prefer-import' }], - '@typescript-eslint/indent': ['off'], - '@typescript-eslint/comma-dangle': ['error', 'always-multiline'], + '@typescript-eslint/non-nullable-type-assertion-style': 'off', '@typescript-eslint/strict-boolean-expressions': 'off', - '@typescript-eslint/member-delimiter-style': [ - 'error', - { - multiline: { - delimiter: 'semi', - requireLast: true, - }, - singleline: { - delimiter: 'semi', - requireLast: false, - }, - }, - ], '@typescript-eslint/explicit-function-return-type': 'off', - 'react/jsx-props-no-spreading': 'warn', - 'react-hooks/rules-of-hooks': 'error', - 'react-hooks/exhaustive-deps': 'warn', + '@typescript-eslint/consistent-type-imports': 'off', + '@typescript-eslint/no-empty-function': 'off', + '@typescript-eslint/no-explicit-any': 'off', 'react/react-in-jsx-scope': 'off', + 'react/prop-types': 'off', + 'react/jsx-props-no-spreading': 'off', }, settings: { react: { diff --git a/samples/apps/copilot-chat-app/webapp/.env.local b/samples/apps/copilot-chat-app/webapp/.env.local deleted file mode 100644 index 1460107535..0000000000 --- a/samples/apps/copilot-chat-app/webapp/.env.local +++ /dev/null @@ -1,2 +0,0 @@ -# Disable ESLint in all environments -DISABLE_ESLINT_PLUGIN=true \ No newline at end of file diff --git a/samples/apps/copilot-chat-app/webapp/package.json b/samples/apps/copilot-chat-app/webapp/package.json index cb46308468..f6fd8dc07e 100644 --- a/samples/apps/copilot-chat-app/webapp/package.json +++ b/samples/apps/copilot-chat-app/webapp/package.json @@ -8,6 +8,7 @@ "auth:mac": "better-vsts-npm-auth -config .npmrc", "depcheck": "depcheck --ignores=\"@types/*,typescript\" --ignore-dirs=\".vscode,.vs,.git,node_modules\" --skip-missing", "lint": "eslint src", + "lint:fix": "eslint src --fix", "prettify": "prettier --write \"src/**/*.{ts,tsx,js,jsx,json,scss,css,html,svg}\"", "serve": "serve -s build", "start": "react-scripts start", @@ -44,10 +45,17 @@ "http-server": "^14.1.1", "prettier": "^2.8.1", "serve": "^14.2.0", - "typescript": "*", + "typescript": "5.0.4", "vsts-npm-auth": "^0.42.1", "workbox-window": "^6.5.4" }, + "eslintConfig": { + "extends": [ + "react-app", + "react-app/jest", + "../../.eslintrc.js" + ] + }, "browserslist": { "production": [ ">0.2%", diff --git a/samples/apps/copilot-chat-app/webapp/src/App.tsx b/samples/apps/copilot-chat-app/webapp/src/App.tsx index 4935ec2cc4..e1fbb32688 100644 --- a/samples/apps/copilot-chat-app/webapp/src/App.tsx +++ b/samples/apps/copilot-chat-app/webapp/src/App.tsx @@ -1,11 +1,6 @@ // Copyright (c) Microsoft. All rights reserved. -import { - AuthenticatedTemplate, - UnauthenticatedTemplate, - useIsAuthenticated, - useMsal, -} from '@azure/msal-react'; +import { AuthenticatedTemplate, UnauthenticatedTemplate, useIsAuthenticated, useMsal } from '@azure/msal-react'; import { Subtitle1, makeStyles, shorthands, tokens } from '@fluentui/react-components'; import { Alert } from '@fluentui/react-components/unstable'; import { Dismiss16Regular } from '@fluentui/react-icons'; @@ -79,21 +74,19 @@ const App: FC = () => { dispatch(setLoggedInUserId(account.homeAccountId)); if (appState === AppState.LoadingChats) { - // Load all chats from the backend. - async function loadChats() { - if (await chat.loadChats()) { + // Load all chats from memory + void chat.loadChats().then((succeeded) => { + if (succeeded) { setAppState(AppState.Chat); } - } - - loadChats(); + }); } } // eslint-disable-next-line react-hooks/exhaustive-deps }, [instance, inProgress, isAuthenticated, appState]); - const onDismissAlert = (key: string) => { - dispatch(removeAlert(key)); + const onDismissAlert = (index: number) => { + dispatch(removeAlert(index)); }; // TODO: handle error case of missing account information @@ -114,34 +107,40 @@ const App: FC = () => { Copilot Chat
- setAppState(AppState.SigningOut)} /> + { + setAppState(AppState.SigningOut); + }} + />
- {alerts && - Object.keys(alerts).map((key) => { - const alert = alerts[key]; - return ( - onDismissAlert(key)} - color="black" - /> - ), - }} - key={key} - > - {alert.message} - - ); - })} + {alerts.map(({ type, message }, index) => { + return ( + { + onDismissAlert(index); + }} + color="black" + /> + ), + }} + key={`${index}-${type}`} + > + {message} + + ); + })} {appState === AppState.ProbeForBackend && ( setAppState(AppState.LoadingChats)} + onBackendFound={() => { + setAppState(AppState.LoadingChats); + }} /> )} {appState === AppState.LoadingChats && } diff --git a/samples/apps/copilot-chat-app/webapp/src/components/FileUploader.tsx b/samples/apps/copilot-chat-app/webapp/src/components/FileUploader.tsx index 474e2e20cd..dbeb3c771e 100644 --- a/samples/apps/copilot-chat-app/webapp/src/components/FileUploader.tsx +++ b/samples/apps/copilot-chat-app/webapp/src/components/FileUploader.tsx @@ -18,7 +18,7 @@ export const FileUploader: React.FC = forwardRef) => { + (event: React.SyntheticEvent) => { const target = event.target as HTMLInputElement; const selectedFiles = target.files; event.stopPropagation(); @@ -50,3 +50,5 @@ export const FileUploader: React.FC = forwardRef; - onSubmit: (options: GetResponseOptions) => void; + onSubmit: (options: GetResponseOptions) => Promise; } export const ChatInput: React.FC = ({ isDraggingOver, onDragLeave, onSubmit }) => { @@ -89,17 +89,21 @@ export const ChatInput: React.FC = ({ isDraggingOver, onDragLeav React.useEffect(() => { async function initSpeechRecognizer() { const speechService = new SpeechService(process.env.REACT_APP_BACKEND_URI as string); - var response = await speechService.getSpeechTokenAsync( + const response = await speechService.getSpeechTokenAsync( await AuthHelper.getSKaaSAccessToken(instance, inProgress), ); if (response.isSuccess) { - const recognizer = await speechService.getSpeechRecognizerAsyncWithValidKey(response); + const recognizer = speechService.getSpeechRecognizerAsyncWithValidKey(response); setRecognizer(recognizer); } } - initSpeechRecognizer(); - }, [instance, inProgress]); + initSpeechRecognizer().catch((e) => { + const errorDetails = e instanceof Error ? e.message : String(e); + const errorMessage = `Unable to initialize speech recognizer. Details: ${errorDetails}`; + dispatch(addAlert({ message: errorMessage, type: AlertType.Error })); + }); + }, [dispatch, instance, inProgress]); React.useEffect(() => { const chatState = conversations[selectedId]; @@ -120,28 +124,30 @@ export const ChatInput: React.FC = ({ isDraggingOver, onDragLeav } }; - const handleImport = async (dragAndDropFile?: File) => { - setDocumentImporting(true); + const handleImport = (dragAndDropFile?: File) => { const file = dragAndDropFile ?? documentFileRef.current?.files?.[0]; if (file) { - await chat.importDocument(selectedId, file); + setDocumentImporting(true); + chat.importDocument(selectedId, file).finally(() => { + setDocumentImporting(false); + }); } - setDocumentImporting(false); // Reset the file input so that the onChange event will // be triggered even if the same file is selected again. - documentFileRef.current!.value = ''; + if (documentFileRef.current?.value) { + documentFileRef.current.value = ''; + } }; const handleSubmit = (value: string, messageType: ChatMessageType = ChatMessageType.Message) => { - try { - if (value.trim() === '') { - return; // only submit if value is not empty - } - onSubmit({ value, messageType, chatId: selectedId }); - setValue(''); - dispatch(editConversationInput({ id: selectedId, newInput: '' })); - } catch (error) { + if (value.trim() === '') { + return; // only submit if value is not empty + } + + setValue(''); + dispatch(editConversationInput({ id: selectedId, newInput: '' })); + onSubmit({ value, messageType, chatId: selectedId }).catch((error) => { const message = `Error submitting chat input: ${(error as Error).message}`; log(message); dispatch( @@ -150,17 +156,19 @@ export const ChatInput: React.FC = ({ isDraggingOver, onDragLeav message, }), ); - } + }); }; - const handleDrop = async (e: React.DragEvent) => { + const handleDrop = (e: React.DragEvent) => { onDragLeave(e); - await handleImport(e.dataTransfer?.files[0]); + handleImport(e.dataTransfer.files[0]); }; return (
-
+
+ +