-
Notifications
You must be signed in to change notification settings - Fork 275
Chat App #134
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
Chat App #134
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| name: Test | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: [ master ] | ||
|
|
||
| jobs: | ||
| build: | ||
|
|
||
| runs-on: ubuntu-latest | ||
|
|
||
| strategy: | ||
| matrix: | ||
| node-version: [20.x] | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v2 | ||
| - name: Use Node.js ${{ matrix.node-version }} | ||
| uses: actions/setup-node@v1 | ||
| with: | ||
| node-version: ${{ matrix.node-version }} | ||
| - run: npm install | ||
| - run: npm test |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| # Logs | ||
| logs | ||
| *.log | ||
| npm-debug.log* | ||
| yarn-debug.log* | ||
| yarn-error.log* | ||
| pnpm-debug.log* | ||
| lerna-debug.log* | ||
|
|
||
| node_modules | ||
| dist | ||
| dist-ssr | ||
| *.local | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider explicitly ignoring environment files to prevent accidental commits of secrets/configuration. Add patterns like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider explicitly ignoring environment files to avoid committing secrets/configuration. Right now you have
|
||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same issue as above — this line appears to contain only a single space character. Remove it to avoid introducing an accidental ignore pattern. |
||
| # Editor directories and files | ||
| .vscode/* | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| !.vscode/extensions.json | ||
| .idea | ||
| .DS_Store | ||
| *.suo | ||
| *.ntvs* | ||
| *.njsproj | ||
| *.sln | ||
| *.sw? | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| # React + TypeScript + Vite | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file is a project README and does not contain any implementation. The task requires an implemented client and server for the chat app (username handling, localStorage, room management, persisted messages). Please provide the source code files (client/src/* and server/*) so those requirements can be validated. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The title and opening line read like a generic Vite + React template. If this README is intended to document the chat client for this project, update the title/intro to describe the chat app and its purpose, not the template. |
||
|
|
||
| This template provides a minimal setup to get React working in Vite with HMR and some ESLint rules. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lines here are documentation about React + Vite and ESLint configuration. They do not implement or document the required server API, socket events, message schema ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This paragraph describes the template. Replace or augment it with a short project summary that lists main features required by the assignment (username persistence in localStorage, rooms CRUD, message history, server communication via HTTP/WS) so reviewers know what to expect. |
||
|
|
||
| Currently, two official plugins are available: | ||
|
|
||
| - [@vitejs/plugin-react](https://github.com/vitejs/vite-plugin-react/blob/main/packages/plugin-react) uses [Babel](https://babeljs.io/) for Fast Refresh | ||
| - [@vitejs/plugin-react-swc](https://github.com/vitejs/vite-plugin-react/blob/main/packages/plugin-react-swc) uses [SWC](https://swc.rs/) for Fast Refresh | ||
|
|
||
| ## Expanding the ESLint configuration | ||
|
|
||
| If you are developing a production application, we recommend updating the configuration to enable type-aware lint rules: | ||
|
Comment on lines
+10
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The "Expanding the ESLint configuration" section is useful for maintainers but distracts from user-facing setup instructions. Consider moving developer-oriented content (ESLint config, plugin setup) to a CONTRIBUTING.md or DEVELOPER.md, and keep README focused on installation and usage. |
||
|
|
||
| ```js | ||
| export default tseslint.config([ | ||
| globalIgnores(['dist']), | ||
| { | ||
| files: ['**/*.{ts,tsx}'], | ||
| extends: [ | ||
| // Other configs... | ||
|
|
||
| // Remove tseslint.configs.recommended and replace with this | ||
| ...tseslint.configs.recommendedTypeChecked, | ||
| // Alternatively, use this for stricter rules | ||
| ...tseslint.configs.strictTypeChecked, | ||
| // Optionally, add this for stylistic rules | ||
| ...tseslint.configs.stylisticTypeChecked, | ||
|
|
||
| // Other configs... | ||
| ], | ||
| languageOptions: { | ||
| parserOptions: { | ||
| project: ['./tsconfig.node.json', './tsconfig.app.json'], | ||
| tsconfigRootDir: import.meta.dirname, | ||
| }, | ||
| // other options... | ||
| }, | ||
| }, | ||
| ]) | ||
| ``` | ||
|
Comment on lines
+14
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This block shows example ESLint config (lines 14-39). While useful, it does not replace the need for the required implementation files. If you intended to submit the code, please attach the implementation files instead of (or in addition to) this README.
Comment on lines
+14
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This tseslint.config code block is a developer toolchain example. If you keep it here, clearly label it as "Developer / Linting configuration" and add a short note telling when/how to use it. Otherwise, move it to a developer guide file to keep the main README concise. |
||
|
|
||
| You can also install [eslint-plugin-react-x](https://github.com/Rel1cx/eslint-react/tree/main/packages/plugins/eslint-plugin-react-x) and [eslint-plugin-react-dom](https://github.com/Rel1cx/eslint-react/tree/main/packages/plugins/eslint-plugin-react-dom) for React-specific lint rules: | ||
|
|
||
| ```js | ||
| // eslint.config.js | ||
| import reactX from 'eslint-plugin-react-x' | ||
| import reactDom from 'eslint-plugin-react-dom' | ||
|
|
||
| export default tseslint.config([ | ||
| globalIgnores(['dist']), | ||
| { | ||
| files: ['**/*.{ts,tsx}'], | ||
| extends: [ | ||
| // Other configs... | ||
| // Enable lint rules for React | ||
| reactX.configs['recommended-typescript'], | ||
| // Enable lint rules for React DOM | ||
| reactDom.configs.recommended, | ||
| ], | ||
| languageOptions: { | ||
| parserOptions: { | ||
| project: ['./tsconfig.node.json', './tsconfig.app.json'], | ||
| tsconfigRootDir: import.meta.dirname, | ||
| }, | ||
| // other options... | ||
| }, | ||
| }, | ||
| ]) | ||
|
Comment on lines
+42
to
+68
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The plugin installation snippet and additional plugin details are also developer-facing. Either move these to a separate developer document or add a dedicated section header (e.g., "For developers: ESLint & plugins") so users aren't confused by advanced configuration details. |
||
| ``` | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This README documents ESLint configuration and plugins — useful. Consider adding a short "How to run" / "Development" section (steps to install deps and start the client, and the expected backend URL/port) so reviewers can quickly launch the client and test the chat app. This will make the project easier to evaluate but does not affect functional requirements. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import js from '@eslint/js' | ||
| import globals from 'globals' | ||
| import reactHooks from 'eslint-plugin-react-hooks' | ||
| import reactRefresh from 'eslint-plugin-react-refresh' | ||
| import tseslint from 'typescript-eslint' | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This import looks suspicious: there is no widely-used package named exactly There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The import on this line uses the module name 'typescript-eslint'. The widely used TypeScript ESLint packages are under the @typescript-eslint scope (for example, '@typescript-eslint/eslint-plugin' and '@typescript-eslint/parser'). Verify that 'typescript-eslint' is an installed package and that it exports what you expect. If not, replace it with the correct package(s) or update how you consume them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Line imports the TypeScript ESLint package using the module name |
||
| import { globalIgnores } from 'eslint/config' | ||
|
|
||
| export default tseslint.config([ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file calls There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file uses |
||
| globalIgnores(['dist']), | ||
|
Comment on lines
+6
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using |
||
| { | ||
| files: ['**/*.{ts,tsx}'], | ||
| extends: [ | ||
| js.configs.recommended, | ||
| tseslint.configs.recommended, | ||
| reactHooks.configs['recommended-latest'], | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verify that the plugin actually exports a |
||
| reactRefresh.configs.vite, | ||
|
Comment on lines
+12
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are using an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Comment on lines
+15
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Double-check that |
||
| ], | ||
|
Comment on lines
+12
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This block uses an export default [ Refactor to compose configs this way rather than relying on |
||
| languageOptions: { | ||
| ecmaVersion: 2020, | ||
| globals: globals.browser, | ||
|
Comment on lines
+18
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Language options set |
||
| }, | ||
|
Comment on lines
+18
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inside your config object you set |
||
| }, | ||
| ]) | ||
|
Comment on lines
+8
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The file currently appears to be a hybrid of legacy and flat ESLint config APIs which will not work as written. Decide which format you want (legacy |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| <!doctype html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8" /> | ||
| <link rel="icon" type="image/svg+xml" href="/vite.svg" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <title>Vite + React + TS</title> | ||
| </head> | ||
| <body> | ||
| <div id="root"></div> | ||
| <script type="module" src="/src/main.tsx"></script> | ||
| </body> | ||
| </html> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line appears to contain only a single space character. That creates a gitignore pattern matching a space character filename (probably unintended). Remove this line to avoid unexpected behavior.