mirror of
https://github.com/coder/coder.git
synced 2026-06-02 20:48:20 +00:00
Bryan
e82e7b3f9f
While I was testing the Sign-out functionality in #46 , I found a bug on login. Sometimes, intermittently, the login wouldn't 'take' even though the login POST was successful and the subsequent GET to `/api/v2/user` was successful - the user would still be brought to the `/login` screen a second time:  ^ I log-in, and then the login screen just resets, and logging in a second time works 🤔 I tracked this down and found it is a new race condition that I inadvertently introduced when I migrated to SWR in #46. Because SWR caches responses based on the API path - we have to invalidate the cache for `/api/v2/user` when the user logins, otherwise we'll continue to serve the user-not-found error until SWR decides to retry. However, we weren't waiting for that cache-invalidation to go through - so there was the chance that an `/api/v2/user` request was still in-flight while we were redirecting after a successful login. If the request made it back prior to the redirect - login would work on the first try. If the request took longer - SWR would serve the stale, erroneous user from the cache. Luckily the fix is simple - `mutate` in the SWR API returns a promise, so we can just wait for that to go through before redirecting. Unfortunately, though, this is tricky to exercise in a unit test because with the SWR model, the login logic is spread between `_app.tsx`, `UserContext.tsx`, and `SignInForm.tsx`, and the race condition involves an integration of all of those places. I found that there is a lint rule that can help protect us from making this mistake - [`typescript-eslint/no-floating-promises`](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/docs/rules/no-floating-promises.md) (it's a lint rule we wanted to turn on in `coder/m`, too, actually - just a lot of work to clean up the code for it!), so I turned it on as part of this PR, and it caught a couple extra places to check.
124 lines
3.7 KiB
YAML
124 lines
3.7 KiB
YAML
---
|
|
env:
|
|
browser: true
|
|
commonjs: true
|
|
es6: true
|
|
jest: true
|
|
node: true
|
|
extends:
|
|
- eslint:recommended
|
|
- plugin:@typescript-eslint/recommended
|
|
- plugin:import/recommended
|
|
- plugin:import/typescript
|
|
- plugin:react/recommended
|
|
- plugin:jsx-a11y/strict
|
|
- plugin:compat/recommended
|
|
- prettier
|
|
parser: "@typescript-eslint/parser"
|
|
parserOptions:
|
|
ecmaVersion: 2018
|
|
project:
|
|
- "./tsconfig.json"
|
|
- "./site/tsconfig.json"
|
|
sourceType: module
|
|
ecmaFeatures:
|
|
jsx: true
|
|
tsconfigRootDir: "./"
|
|
plugins:
|
|
- "@typescript-eslint"
|
|
- import
|
|
- react-hooks
|
|
- jest
|
|
- no-storage
|
|
root: true
|
|
rules:
|
|
"@typescript-eslint/brace-style":
|
|
["error", "1tbs", { "allowSingleLine": false }]
|
|
"@typescript-eslint/camelcase": "off"
|
|
"@typescript-eslint/explicit-function-return-type": "off"
|
|
"@typescript-eslint/explicit-module-boundary-types": "error"
|
|
"@typescript-eslint/method-signature-style": ["error", "property"]
|
|
"@typescript-eslint/no-floating-promises": error
|
|
"@typescript-eslint/no-invalid-void-type": error
|
|
# We're disabling the `no-namespace` rule to use a pattern of defining an interface,
|
|
# and then defining functions that operate on that data via namespace. This is helpful for
|
|
# dealing with immutable objects. This is a common pattern that shows up in some other
|
|
# large TypeScript projects, like VSCode.
|
|
# More details: https://github.com/coder/m/pull/9720#discussion_r697609528
|
|
"@typescript-eslint/no-namespace": "off"
|
|
"@typescript-eslint/no-unnecessary-boolean-literal-compare": error
|
|
"@typescript-eslint/no-unnecessary-condition": warn
|
|
"@typescript-eslint/no-unnecessary-type-assertion": warn
|
|
"@typescript-eslint/no-unused-vars":
|
|
- error
|
|
- argsIgnorePattern: "^_"
|
|
varsIgnorePattern: "^_"
|
|
"@typescript-eslint/no-use-before-define": "off"
|
|
"@typescript-eslint/object-curly-spacing": ["error", "always"]
|
|
"@typescript-eslint/triple-slash-reference": "off"
|
|
"brace-style": "off"
|
|
"curly": ["error", "all"]
|
|
eqeqeq: error
|
|
import/default: "off"
|
|
import/namespace: "off"
|
|
import/newline-after-import:
|
|
- error
|
|
- count: 1
|
|
import/no-named-as-default: "off"
|
|
import/no-named-as-default-member: "off"
|
|
import/prefer-default-export: "off"
|
|
jest/no-focused-tests: "error"
|
|
jsx-a11y/label-has-for: "off"
|
|
jsx-a11y/no-autofocus: "off"
|
|
no-console:
|
|
- warn
|
|
- allow:
|
|
- warn
|
|
- error
|
|
- info
|
|
- debug
|
|
no-dupe-class-members: "off"
|
|
no-restricted-imports:
|
|
- error
|
|
- paths:
|
|
- name: "@material-ui/core"
|
|
message:
|
|
"Use path imports to avoid pulling in unused modules. See:
|
|
https://material-ui.com/guides/minimizing-bundle-size/"
|
|
- name: "@material-ui/icons"
|
|
message:
|
|
"Use path imports to avoid pulling in unused modules. See:
|
|
https://material-ui.com/guides/minimizing-bundle-size/"
|
|
- name: "@material-ui/styles"
|
|
message:
|
|
"Use path imports to avoid pulling in unused modules. See:
|
|
https://material-ui.com/guides/minimizing-bundle-size/"
|
|
- name: "@material-ui/core/Tooltip"
|
|
message: "Use the custom Tooltip on componens/Tooltip"
|
|
no-storage/no-browser-storage: error
|
|
no-unused-vars: "off"
|
|
"object-curly-spacing": "off"
|
|
react-hooks/exhaustive-deps: warn
|
|
react-hooks/rules-of-hooks: error
|
|
react/jsx-no-script-url:
|
|
- error
|
|
- - name: Link
|
|
props:
|
|
- to
|
|
- name: Button
|
|
props:
|
|
- href
|
|
- name: IconButton
|
|
props:
|
|
- href
|
|
react/prop-types: "off"
|
|
react/jsx-boolean-value: ["error", "never"]
|
|
react/jsx-curly-brace-presence:
|
|
- error
|
|
- children: ignore
|
|
settings:
|
|
react:
|
|
version: detect
|
|
import/resolver:
|
|
typescript: {}
|