From 2fa9b424f944103f53b5bd9ae38dfc7635528dc4 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 8 Jan 2025 21:38:47 -0600 Subject: [PATCH 1/6] remove stray console.log in table header --- app/table/Table.tsx | 45 +++++++++++++++++++++------------------------ 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/app/table/Table.tsx b/app/table/Table.tsx index 266ae51805..22415c09c3 100644 --- a/app/table/Table.tsx +++ b/app/table/Table.tsx @@ -29,30 +29,27 @@ export const Table = ({ }: TableProps) => ( - {table.getHeaderGroups().map((headerGroup) => { - console.log(headerGroup) - return ( - - {headerGroup.headers.map((header) => ( - - { - // Placeholder concept is for when grouped columns are - // combined with regular columns. The regular column only - // needs one entry in the stack of header cells, so the others - // have isPlacholder=true. See sleds table for an example. - header.isPlaceholder - ? null - : flexRender(header.column.columnDef.header, header.getContext()) - } - - ))} - - ) - })} + {table.getHeaderGroups().map((headerGroup) => ( + + {headerGroup.headers.map((header) => ( + + { + // Placeholder concept is for when grouped columns are + // combined with regular columns. The regular column only + // needs one entry in the stack of header cells, so the others + // have isPlacholder=true. See sleds table for an example. + header.isPlaceholder + ? null + : flexRender(header.column.columnDef.header, header.getContext()) + } + + ))} + + ))} {table.getRowModel().rows.map((row) => { From a6f84049118c09bd708f32cc5042ffe31f7388b6 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 9 Jan 2025 11:42:43 -0600 Subject: [PATCH 2/6] Lint for stray `console.log`s (#2643) * lint for stray console.logs * upgrade oxlint, add oxlint npm run script --- .eslintrc.cjs | 2 ++ app/api/hooks.ts | 2 +- app/api/window.ts | 2 +- app/forms/image-upload.tsx | 2 +- app/msw-mock-api.ts | 4 +-- package-lock.json | 72 +++++++++++++++++++------------------- package.json | 4 +-- 7 files changed, 45 insertions(+), 43 deletions(-) diff --git a/.eslintrc.cjs b/.eslintrc.cjs index 396b301225..fdea7f935f 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -70,6 +70,8 @@ module.exports = { 'import/no-default-export': 'error', 'import/no-unresolved': 'off', // plugin doesn't know anything 'jsx-a11y/label-has-associated-control': [2, { controlComponents: ['button'] }], + // only worry about console.log + 'no-console': ['error', { allow: ['warn', 'error', 'info', 'table'] }], 'no-param-reassign': 'error', 'no-restricted-imports': [ 'error', diff --git a/app/api/hooks.ts b/app/api/hooks.ts index 23a374c54a..66773b0120 100644 --- a/app/api/hooks.ts +++ b/app/api/hooks.ts @@ -346,7 +346,7 @@ export const wrapQueryClient = (api: A, queryClient: QueryC // directly without further processing throw data } - console.log(options.explanation) + console.info(options.explanation) return { type: 'error' as const, data } }), ...options, diff --git a/app/api/window.ts b/app/api/window.ts index 0a983f1896..72e910e41d 100644 --- a/app/api/window.ts +++ b/app/api/window.ts @@ -34,7 +34,7 @@ function handleResult(result: ApiResult): T { } function logHeading(s: string) { - console.log(`%c${s}`, 'font-size: 16px; font-weight: bold;') + console.info(`%c${s}`, 'font-size: 16px; font-weight: bold;') } if (typeof window !== 'undefined') { diff --git a/app/forms/image-upload.tsx b/app/forms/image-upload.tsx index 0b85ba3361..9a77b95dd6 100644 --- a/app/forms/image-upload.tsx +++ b/app/forms/image-upload.tsx @@ -515,7 +515,7 @@ export function Component() { .catch((e) => { // eat a 404 since that's what we want. anything else should still blow up if (e.statusCode === 404) { - console.log( + console.info( '/v1/images 404 is expected. It means the image name is not taken.' ) return null diff --git a/app/msw-mock-api.ts b/app/msw-mock-api.ts index fe8ac25847..2aa6fc5b61 100644 --- a/app/msw-mock-api.ts +++ b/app/msw-mock-api.ts @@ -15,7 +15,7 @@ function getChaos() { const chaos = getChaos() if (process.env.NODE_ENV !== 'production' && chaos) { - console.log(` + console.info(` ▄████████ ▄█ █▄ ▄████████ ▄██████▄ ▄████████ ███ ███ ███ ███ ███ ███ ███ ███ ███ ███ ███ █▀ ███ ███ ███ ███ ███ ███ ███ █▀ @@ -34,7 +34,7 @@ if (process.env.NODE_ENV !== 'production' && chaos) { ███ ███ ███ ███ ███ ███ ▄███ ███ ███ ▀█ ███ █▀ ▀██████▀ ████████▀ ██████████ `) - console.log(`Running MSW in CHAOS MODE with ${chaos}% likelihood of random failure`) + console.info(`Running MSW in CHAOS MODE with ${chaos}% likelihood of random failure`) } /** Return true for failure with a given likelihood */ diff --git a/package-lock.json b/package-lock.json index 5159e7bc15..bd20cf4668 100644 --- a/package-lock.json +++ b/package-lock.json @@ -87,7 +87,7 @@ "ip-num": "^1.5.1", "jsdom": "^25.0.1", "msw": "^2.7.0", - "oxlint": "^0.14.1", + "oxlint": "^0.15.5", "patch-package": "^8.0.0", "postcss": "^8.4.49", "postcss-import": "^16.1.0", @@ -1656,9 +1656,9 @@ } }, "node_modules/@oxlint/darwin-arm64": { - "version": "0.14.1", - "resolved": "https://registry.npmjs.org/@oxlint/darwin-arm64/-/darwin-arm64-0.14.1.tgz", - "integrity": "sha512-P76G8QCHOkLm+8HYk2/5uR4sPnx6uxE5Y8ik8dgCV0XjrNR+/Sg8v2aQ1BmWeEnPGkBXTt1VSECO9BdT1HVeDw==", + "version": "0.15.5", + "resolved": "https://registry.npmjs.org/@oxlint/darwin-arm64/-/darwin-arm64-0.15.5.tgz", + "integrity": "sha512-LqUh/+iRwj2mIDMqiYOjf2nx/BjpMVY+2PQvpOgSd2mNXrudD02KUTNCVSTAhUuGe86SYQblFgBz2cYfbTUa9A==", "cpu": [ "arm64" ], @@ -1670,9 +1670,9 @@ ] }, "node_modules/@oxlint/darwin-x64": { - "version": "0.14.1", - "resolved": "https://registry.npmjs.org/@oxlint/darwin-x64/-/darwin-x64-0.14.1.tgz", - "integrity": "sha512-bFsNkDtiDEhBKsX2DGLGCVhaRSDP2VgnNHOejjVnLK2LURvOglHMrp4NXxxtHArPAfiP6oezja6q8GmsQbcZ4w==", + "version": "0.15.5", + "resolved": "https://registry.npmjs.org/@oxlint/darwin-x64/-/darwin-x64-0.15.5.tgz", + "integrity": "sha512-3AdgAKrZsZ1Tk4XW5GosNKfRk5tdAzYgZl/9lJSCJr2I5mN6hrTn/WTIJv4tJrLlQbFsD++5R5HCQblp9vfFNg==", "cpu": [ "x64" ], @@ -1684,9 +1684,9 @@ ] }, "node_modules/@oxlint/linux-arm64-gnu": { - "version": "0.14.1", - "resolved": "https://registry.npmjs.org/@oxlint/linux-arm64-gnu/-/linux-arm64-gnu-0.14.1.tgz", - "integrity": "sha512-OWJY1qxJgsaLyQvh97MdpI2Mr8FD90ssGw8o0rG63lWIc3PJESmq9NKU0ZwwUbPbbEpKmwdG3aiZRjh4G1k0cQ==", + "version": "0.15.5", + "resolved": "https://registry.npmjs.org/@oxlint/linux-arm64-gnu/-/linux-arm64-gnu-0.15.5.tgz", + "integrity": "sha512-iKqoRSn+aIYqku99xBFzhZFaRXTIpFhFIh37MFjzhPvvVN5qQjxBzvePfuI1eTVuttviELKP5eR5sSUP2aDKxw==", "cpu": [ "arm64" ], @@ -1698,9 +1698,9 @@ ] }, "node_modules/@oxlint/linux-arm64-musl": { - "version": "0.14.1", - "resolved": "https://registry.npmjs.org/@oxlint/linux-arm64-musl/-/linux-arm64-musl-0.14.1.tgz", - "integrity": "sha512-GPbggyGQV4+5JzpA7l+1liPHkzCDB9ZyPLcHpRtNJJfTQ60JnBww4l3eR7LCukiAor6Sxmlbl+t1OZGjL3zUUA==", + "version": "0.15.5", + "resolved": "https://registry.npmjs.org/@oxlint/linux-arm64-musl/-/linux-arm64-musl-0.15.5.tgz", + "integrity": "sha512-pYNCI9iqPcma8hIuJb/v40mOjuWtJjYaOxbDvSecjtKb9EFqZu9A9WDhgg766S+8o1Iit72LLtSq7q0cyh7+ww==", "cpu": [ "arm64" ], @@ -1712,9 +1712,9 @@ ] }, "node_modules/@oxlint/linux-x64-gnu": { - "version": "0.14.1", - "resolved": "https://registry.npmjs.org/@oxlint/linux-x64-gnu/-/linux-x64-gnu-0.14.1.tgz", - "integrity": "sha512-4ug2y9fEa2MB4wAFfITkm1oJ2m14YjWQaKxKN9bRazPng2k3wivVAvwc6tKj866HftZxXo3FlOIrE1YP6BxcSw==", + "version": "0.15.5", + "resolved": "https://registry.npmjs.org/@oxlint/linux-x64-gnu/-/linux-x64-gnu-0.15.5.tgz", + "integrity": "sha512-jDrA5vSdEhKov1aMFvWLzFpuGneVhryLXTqcbqdUzZOe1Ss1S2XhRabHwLlhDRemCA4Id7fdgZ5qCNrzhUsTdQ==", "cpu": [ "x64" ], @@ -1726,9 +1726,9 @@ ] }, "node_modules/@oxlint/linux-x64-musl": { - "version": "0.14.1", - "resolved": "https://registry.npmjs.org/@oxlint/linux-x64-musl/-/linux-x64-musl-0.14.1.tgz", - "integrity": "sha512-bsG5ZxFWKml6BQMHbusvNsEU3O0a5BurlrtdXyxlOBZyrWyG6v3pcXM+NX4YT8gaeoK71iCH5I+ymwI9KOwO/w==", + "version": "0.15.5", + "resolved": "https://registry.npmjs.org/@oxlint/linux-x64-musl/-/linux-x64-musl-0.15.5.tgz", + "integrity": "sha512-eS4ANgxQCNnGG1aeQ28cUTr/3iitFF9TBK1BfTn3+WW3Nvkfcxo+u2L5tKEq+cxytHuu9hqEpTn1AvNyAH3MQw==", "cpu": [ "x64" ], @@ -1740,9 +1740,9 @@ ] }, "node_modules/@oxlint/win32-arm64": { - "version": "0.14.1", - "resolved": "https://registry.npmjs.org/@oxlint/win32-arm64/-/win32-arm64-0.14.1.tgz", - "integrity": "sha512-Soc3kRTqz++kleXz+Y4IlfiGY+cwXlOiLCVwcRMAnY+TSaP4h2tPXN/cbOypsE7PPq2/kk7JPGUaEKZ/i4G23A==", + "version": "0.15.5", + "resolved": "https://registry.npmjs.org/@oxlint/win32-arm64/-/win32-arm64-0.15.5.tgz", + "integrity": "sha512-oJPS+dTlwgVoSb6ieH/4MP+LPxZUMPsUr5k0YLpisMEEfccGXLaGzWKfzgi5dzlOsN6EUr2NgcFaqpa8tRiq8w==", "cpu": [ "arm64" ], @@ -1754,9 +1754,9 @@ ] }, "node_modules/@oxlint/win32-x64": { - "version": "0.14.1", - "resolved": "https://registry.npmjs.org/@oxlint/win32-x64/-/win32-x64-0.14.1.tgz", - "integrity": "sha512-qyjlv5XPxKJ1g9F4ZpNP0m/I2tgHj4lebmyAveaLos3RZWut983WaK3abq4Mr74mIiwfyLA67/m2khG5aXYN2Q==", + "version": "0.15.5", + "resolved": "https://registry.npmjs.org/@oxlint/win32-x64/-/win32-x64-0.15.5.tgz", + "integrity": "sha512-rvk3CN0U37w7OlLvkD1TqqcLvoQdWVtTAF5KckuASOeYo/0cuzNFIvo/7cHYmvewQXuYULxvUHGTzxWYSmVPjQ==", "cpu": [ "x64" ], @@ -11168,9 +11168,9 @@ } }, "node_modules/oxlint": { - "version": "0.14.1", - "resolved": "https://registry.npmjs.org/oxlint/-/oxlint-0.14.1.tgz", - "integrity": "sha512-FwcjPfQu806ibSv73Y9tUM8ezUyd811dp3JwEEOC/dIAgd93egRsRNnFauuAq/WuTjIDv73tbr9hB8MziH31Eg==", + "version": "0.15.5", + "resolved": "https://registry.npmjs.org/oxlint/-/oxlint-0.15.5.tgz", + "integrity": "sha512-hpckVhX+oiDyxYk7rwHqxZAmtOAQxQbkvxGM9NTQ3dTQC8X/5zcoBA9cZdrjVENA2TXrjg7l2qmkkzX7seteoQ==", "dev": true, "license": "MIT", "bin": { @@ -11184,14 +11184,14 @@ "url": "https://github.com/sponsors/Boshen" }, "optionalDependencies": { - "@oxlint/darwin-arm64": "0.14.1", - "@oxlint/darwin-x64": "0.14.1", - "@oxlint/linux-arm64-gnu": "0.14.1", - "@oxlint/linux-arm64-musl": "0.14.1", - "@oxlint/linux-x64-gnu": "0.14.1", - "@oxlint/linux-x64-musl": "0.14.1", - "@oxlint/win32-arm64": "0.14.1", - "@oxlint/win32-x64": "0.14.1" + "@oxlint/darwin-arm64": "0.15.5", + "@oxlint/darwin-x64": "0.15.5", + "@oxlint/linux-arm64-gnu": "0.15.5", + "@oxlint/linux-arm64-musl": "0.15.5", + "@oxlint/linux-x64-gnu": "0.15.5", + "@oxlint/linux-x64-musl": "0.15.5", + "@oxlint/win32-arm64": "0.15.5", + "@oxlint/win32-x64": "0.15.5" } }, "node_modules/p-limit": { diff --git a/package.json b/package.json index 4dc0dbeab8..56f18247f7 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "e2e": "playwright test", "e2ec": "playwright test --project=chrome", "lint": "eslint --ext .js,.ts,.tsx app test mock-api", - "lint-fast": "npm run lint -- --cache", + "oxlint": "oxlint", "fmt": "prettier --cache --write . && npm run lint -- --fix", "openapi-gen-ts": "openapi-gen-ts", "prettier": "prettier", @@ -109,7 +109,7 @@ "ip-num": "^1.5.1", "jsdom": "^25.0.1", "msw": "^2.7.0", - "oxlint": "^0.14.1", + "oxlint": "^0.15.5", "patch-package": "^8.0.0", "postcss": "^8.4.49", "postcss-import": "^16.1.0", From a3e8216e7040f6b31850a6b3b3243bf30c6d0803 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 9 Jan 2025 11:44:37 -0600 Subject: [PATCH 3/6] tools: also lint for console.log in oxlint --- .oxlintrc.json | 5 ++++- tools/deno/api-diff.ts | 4 ++-- tools/deno/bump-omicron.ts | 22 +++++++++++----------- tools/deno/deploy-dogfood.ts | 16 ++++++++-------- tools/deno/mock-serial-console.ts | 4 ++-- tools/start_mock_api.ts | 2 +- 6 files changed, 28 insertions(+), 25 deletions(-) diff --git a/.oxlintrc.json b/.oxlintrc.json index 00f8ad504a..a678af9513 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -3,6 +3,8 @@ "plugins": ["react", "react-hooks", "unicorn", "typescript", "oxc"], "rules": { "react-hooks/exhaustive-deps": "error", + // only worry about console.log + "no-console": ["error", { "allow": ["warn", "error", "info", "table"] }], // turning this off because it's more sensitive than eslint currently // "react-hooks/rules-of-hooks": "error", "no-unused-vars": [ @@ -13,5 +15,6 @@ "caughtErrorsIgnorePattern": "^_" } ] - } + }, + "ignorePatterns": ["dist/", "node_modules/"] } diff --git a/tools/deno/api-diff.ts b/tools/deno/api-diff.ts index 1c1d4a6d48..8a968e6329 100755 --- a/tools/deno/api-diff.ts +++ b/tools/deno/api-diff.ts @@ -34,7 +34,7 @@ Parameters: `.trim() function printHelpAndExit(): never { - console.log(HELP) + console.info(HELP) Deno.exit() } @@ -98,7 +98,7 @@ async function genForCommit(commit: string, force: boolean) { if (force || !alreadyExists) { await $`rm -rf ${tmpDir}` await $`mkdir -p ${tmpDir}` - console.log(`Generating for ${commit}...`) + console.info(`Generating for ${commit}...`) await $`npx @oxide/openapi-gen-ts@0.4.0 ${specUrl(commit)} ${tmpDir}` await $`npx prettier --write --log-level error ${tmpDir}` } diff --git a/tools/deno/bump-omicron.ts b/tools/deno/bump-omicron.ts index b9d3b5add1..1f639f9685 100755 --- a/tools/deno/bump-omicron.ts +++ b/tools/deno/bump-omicron.ts @@ -65,7 +65,7 @@ const args = flags.parse(Deno.args, { }) if (args.help) { - console.log(HELP) + console.info(HELP) Deno.exit() } @@ -103,7 +103,7 @@ const oldCommit = /COMMIT="?([a-f0-9]+)"?/.exec(oldVersionFile)?.[1] if (!oldCommit) throw Error('Could not parse existing version file') if (oldCommit === newCommit) { - console.log('Nothing to update: Omicron already has the current commit pinned') + console.info('Nothing to update: Omicron already has the current commit pinned') Deno.exit() } @@ -118,7 +118,7 @@ const changesLine = `https://github.com/oxidecomputer/console/compare/${commitRa const branchName = 'bump-console-' + newCommit.slice(0, 8) const prBody = `${changesLine}\n\n${commitsMarkdown}` -console.log(`\n${changesLine}\n\n${commits}\n`) +console.info(`\n${changesLine}\n\n${commits}\n`) if (args.dryRun) Deno.exit() @@ -131,7 +131,7 @@ const message = const prTitle = 'Bump web console' + (message ? ` (${message})` : '') -console.log(`\nPR title: ${prTitle}\n`) +console.info(`\nPR title: ${prTitle}\n`) const go = await $.confirm({ message: 'Make Omicron PR?', noClear: true }) if (!go) Deno.exit() @@ -139,7 +139,7 @@ if (!go) Deno.exit() if (!$.commandExistsSync('gh')) throw Error(GH_MISSING) await Deno.writeTextFile(VERSION_FILE, newVersionFile) -console.log('Updated ', VERSION_FILE) +console.info('Updated ', VERSION_FILE) const consoleDir = Deno.cwd() @@ -149,28 +149,28 @@ Deno.chdir(OMICRON_DIR) await $`git checkout main` await $`git pull` await $`git checkout -b ${branchName}` -console.log('Created branch', branchName) +console.info('Created branch', branchName) await $`git add tools/console_version` await $`git commit -m ${prTitle} -m ${prBody}` await $`git push --set-upstream origin ${branchName}` -console.log('Committed changes and pushed') +console.info('Committed changes and pushed') // create PR const prUrl = await $`gh pr create --title ${prTitle} --body ${prBody}`.text() -console.log('PR created:', prUrl) +console.info('PR created:', prUrl) // set it to auto merge const prNum = prUrl.match(/\d+$/)![0] await $`gh pr merge ${prNum} --auto --squash` -console.log('PR set to auto-merge when CI passes') +console.info('PR set to auto-merge when CI passes') await $`git checkout main` await $`git branch -D ${branchName}` -console.log('Checked out omicron main, deleted branch', branchName) +console.info('Checked out omicron main, deleted branch', branchName) // bump omicron tag in console to current commit Deno.chdir(consoleDir) -console.log('Bumping omicron tag in console') +console.info('Bumping omicron tag in console') await $`git tag -f -a omicron -m 'pinned commit on omicron main'` await $`git push -f origin tag omicron` diff --git a/tools/deno/deploy-dogfood.ts b/tools/deno/deploy-dogfood.ts index 0a861978a3..4f1f814af0 100755 --- a/tools/deno/deploy-dogfood.ts +++ b/tools/deno/deploy-dogfood.ts @@ -36,36 +36,36 @@ const consoleCommit = args._[0] if (!consoleCommit) { console.error('Error: Console commit hash is required\n') - console.log(USAGE) + console.info(USAGE) Deno.exit(1) } -console.log('Finding nexus zones...') +console.info('Finding nexus zones...') const zones: string = await $`./tools/dogfood/find-zone.sh nexus`.text() const gimletNums = zones .split('\n') .filter((line) => line.includes('nexus')) .map((line) => line.trim().split(' ')[0]) -console.log(`Found: ${JSON.stringify(gimletNums)}\n`) +console.info(`Found: ${JSON.stringify(gimletNums)}\n`) const TARBALL_URL = `https://dl.oxide.computer/releases/console/${consoleCommit}.tar.gz` const TARBALL_FILE = '/tmp/console.tar.gz' -console.log(`Downloading tarball to ${TARBALL_FILE}`) +console.info(`Downloading tarball to ${TARBALL_FILE}`) await $`curl --show-error --fail --location --output ${TARBALL_FILE} ${TARBALL_URL}` -console.log(`Done downloading.\n`) +console.info(`Done downloading.\n`) const go = await $.confirm(`Deploy console to gimlets ${JSON.stringify(gimletNums)}?`) if (!go) { - console.log('Deploy aborted') + console.info('Deploy aborted') Deno.exit() } async function deploy(num: string) { - console.log(`Deploying to gimlet ${num}...`) + console.info(`Deploying to gimlet ${num}...`) await $`./tools/dogfood/scp-assets.sh gc${num} ${TARBALL_FILE}` - console.log(`Done with gimlet ${num}...`) + console.info(`Done with gimlet ${num}...`) } await Promise.all(gimletNums.map(deploy)) diff --git a/tools/deno/mock-serial-console.ts b/tools/deno/mock-serial-console.ts index aece68b8bf..3796bffca8 100755 --- a/tools/deno/mock-serial-console.ts +++ b/tools/deno/mock-serial-console.ts @@ -27,7 +27,7 @@ async function serialConsole(req: Request) { const { socket, response } = Deno.upgradeWebSocket(req) socket.binaryType = 'arraybuffer' - console.log(`New client connected`) + console.info(`New client connected`) // send hello as a binary frame for xterm to display socket.onopen = () => { @@ -39,7 +39,7 @@ async function serialConsole(req: Request) { // echo back binary data socket.onmessage = (m) => socket.send(m.data) - socket.onclose = () => console.log('Connection closed') + socket.onclose = () => console.info('Connection closed') return response } diff --git a/tools/start_mock_api.ts b/tools/start_mock_api.ts index a6bcdd8f54..75786309b5 100644 --- a/tools/start_mock_api.ts +++ b/tools/start_mock_api.ts @@ -12,7 +12,7 @@ import { handlers } from '../mock-api/msw/handlers' // TODO: take port argument createServer(...handlers).listen(12220) -console.log('Mock Nexus API running at http://localhost:12220') +console.info('Mock Nexus API running at http://localhost:12220') // TODO: request logging. I tried adding this as a full express server with // logging middleware and it only logged 404s, not good requests From b55e1618a1f9b33b12210803a871948da93b8e5a Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 9 Jan 2025 12:20:05 -0600 Subject: [PATCH 4/6] Remove format action (#2642) * remove reformatter github workflow * introduce formatting error to test linter in CI * Back out "introduce formatting error to test linter in CI" This backs out commit 3b18a8caa13e7ff8a7c39aff82ae3d85690ffaa0. --- .github/workflows/reformatter.yaml | 30 ------------------------------ 1 file changed, 30 deletions(-) delete mode 100644 .github/workflows/reformatter.yaml diff --git a/.github/workflows/reformatter.yaml b/.github/workflows/reformatter.yaml deleted file mode 100644 index 15004f92c7..0000000000 --- a/.github/workflows/reformatter.yaml +++ /dev/null @@ -1,30 +0,0 @@ -name: Reformat - -on: - push: - branches-ignore: - - main - -jobs: - ci: - runs-on: ubuntu-latest - - steps: - - uses: actions/checkout@v4 - - - uses: actions/setup-node@v4 - with: - node-version: 22 - cache: 'npm' - - - name: Install dependencies - run: npm install - - - name: Format - run: npm run fmt - - - uses: EndBug/add-and-commit@v9 - with: - add: . - message: 'Bot commit: format with prettier' - default_author: github_actions From b57531d9fb9e88d7987afce35954742ed4ce7a67 Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Fri, 10 Jan 2025 05:47:42 +0000 Subject: [PATCH 5/6] =?UTF-8?q?Updated=20=E2=80=93=20Switching=20from=20`@?= =?UTF-8?q?react-spring/web`=20to=20`motion`=20(#2646)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Switch from `react-spring` to `motion` * Handle reduced motion automatically * Improved close icon alignment and hover state * actually lazy load the animation code. 18kb gz savings --------- Co-authored-by: David Crespo --- app/components/RoundedSector.tsx | 3 +- app/components/ToastStack.tsx | 48 ++++---- app/hooks/use-reduce-motion.tsx | 38 ------ app/main.tsx | 19 +-- app/ui/lib/Button.tsx | 34 +++++- app/ui/lib/CopyToClipboard.tsx | 43 ++++--- app/ui/lib/DialogOverlay.tsx | 7 +- app/ui/lib/Modal.tsx | 96 +++++++-------- app/ui/lib/SideModal.tsx | 132 +++++++++------------ app/ui/lib/TimeoutIndicator.tsx | 26 +---- app/ui/lib/Toast.tsx | 37 +++--- app/ui/styles/components/menu-button.css | 2 +- app/ui/styles/index.css | 38 ++++++ app/util/motion-features.ts | 10 ++ package-lock.json | 142 +++++++++++------------ package.json | 2 +- 16 files changed, 329 insertions(+), 348 deletions(-) delete mode 100644 app/hooks/use-reduce-motion.tsx create mode 100644 app/util/motion-features.ts diff --git a/app/components/RoundedSector.tsx b/app/components/RoundedSector.tsx index abf27bcd18..960dd9f539 100644 --- a/app/components/RoundedSector.tsx +++ b/app/components/RoundedSector.tsx @@ -5,10 +5,9 @@ * * Copyright Oxide Computer Company */ +import { useReducedMotion } from 'motion/react' import { useEffect, useMemo, useState } from 'react' -import { useReducedMotion } from '~/hooks/use-reduce-motion' - export function RoundedSector({ angle, size, diff --git a/app/components/ToastStack.tsx b/app/components/ToastStack.tsx index 78fe655e69..39a95919e7 100644 --- a/app/components/ToastStack.tsx +++ b/app/components/ToastStack.tsx @@ -5,7 +5,8 @@ * * Copyright Oxide Computer Company */ -import { animated, useTransition } from '@react-spring/web' +import { AnimatePresence } from 'motion/react' +import * as m from 'motion/react-m' import { removeToast, useToastStore } from '~/stores/toast' import { Toast } from '~/ui/lib/Toast' @@ -13,37 +14,30 @@ import { Toast } from '~/ui/lib/Toast' export function ToastStack() { const toasts = useToastStore((state) => state.toasts) - const transition = useTransition(toasts, { - keys: (toast) => toast.id, - from: { opacity: 0, y: 10, scale: 95 }, - enter: { opacity: 1, y: 0, scale: 100 }, - leave: { opacity: 0, y: 10, scale: 95 }, - config: { duration: 100 }, - }) - return (
- {transition((style, item) => ( - `scale(${val}%, ${val}%)`), - }} - > - { - removeToast(item.id) - item.options.onClose?.() - }} - /> - - ))} + + {toasts.map((toast) => ( + + { + removeToast(toast.id) + toast.options.onClose?.() + }} + /> + + ))} +
) } diff --git a/app/hooks/use-reduce-motion.tsx b/app/hooks/use-reduce-motion.tsx deleted file mode 100644 index e5db389868..0000000000 --- a/app/hooks/use-reduce-motion.tsx +++ /dev/null @@ -1,38 +0,0 @@ -/* - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, you can obtain one at https://mozilla.org/MPL/2.0/. - * - * Copyright Oxide Computer Company - */ -import { Globals } from '@react-spring/web' -import { useEffect, useState } from 'react' - -Globals.assign({ skipAnimation: true }) - -const motionQuery = () => window.matchMedia('(prefers-reduced-motion: reduce)') - -/** - * Pulled from [react-reduce-motion](https://github.com/infiniteluke/react-reduce-motion). - */ -export function useReducedMotion() { - const [reducedMotion, setReducedMotion] = useState(motionQuery().matches) - useEffect(() => { - const mq = motionQuery() - const handleChange = () => setReducedMotion(mq.matches) - handleChange() - mq.addEventListener('change', handleChange) - return () => mq.removeEventListener('change', handleChange) - }, []) - return reducedMotion -} - -export function ReduceMotion() { - const prefersReducedMotion = useReducedMotion() - - useEffect(() => { - Globals.assign({ skipAnimation: prefersReducedMotion }) - }, [prefersReducedMotion]) - - return null -} diff --git a/app/main.tsx b/app/main.tsx index 1db612059e..6bc7ecb541 100644 --- a/app/main.tsx +++ b/app/main.tsx @@ -6,6 +6,7 @@ * Copyright Oxide Computer Company */ import { QueryClientProvider } from '@tanstack/react-query' +import { LazyMotion, MotionConfig } from 'motion/react' // import { ReactQueryDevtools } from '@tanstack/react-query-devtools' import { StrictMode } from 'react' import { createRoot } from 'react-dom/client' @@ -16,7 +17,6 @@ import { queryClient } from '@oxide/api' import { ConfirmActionModal } from './components/ConfirmActionModal' import { ErrorBoundary } from './components/ErrorBoundary' -import { ReduceMotion } from './hooks/use-reduce-motion' // stripped out by rollup in production import { startMockAPI } from './msw-mock-api' import { routes } from './routes' @@ -33,6 +33,8 @@ if (process.env.SHA) { ) } +const loadFeatures = () => import('./util/motion-features').then((res) => res.domAnimation) + const root = createRoot(document.getElementById('root')!) function render() { @@ -46,12 +48,15 @@ function render() { root.render( - - - - - - + + + + + + + + + {/* */} diff --git a/app/ui/lib/Button.tsx b/app/ui/lib/Button.tsx index d77c893b6d..b5a3190fe7 100644 --- a/app/ui/lib/Button.tsx +++ b/app/ui/lib/Button.tsx @@ -6,6 +6,7 @@ * Copyright Oxide Computer Company */ import cn from 'classnames' +import * as m from 'motion/react-m' import { forwardRef, type MouseEventHandler, type ReactNode } from 'react' import { Spinner } from '~/ui/lib/Spinner' @@ -90,9 +91,14 @@ export const Button = forwardRef( with={} > ) diff --git a/app/ui/lib/CopyToClipboard.tsx b/app/ui/lib/CopyToClipboard.tsx index da07926d4e..df0331f531 100644 --- a/app/ui/lib/CopyToClipboard.tsx +++ b/app/ui/lib/CopyToClipboard.tsx @@ -6,8 +6,9 @@ * Copyright Oxide Computer Company */ -import { animated, config, useTransition } from '@react-spring/web' import cn from 'classnames' +import { AnimatePresence } from 'motion/react' +import * as m from 'motion/react-m' import { useState } from 'react' import { Copy12Icon, Success12Icon } from '@oxide/design-system/icons/react' @@ -20,6 +21,11 @@ type Props = { className?: string } +const variants = { + hidden: { opacity: 0, scale: 0.75 }, + visible: { opacity: 1, scale: 1 }, +} + export const CopyToClipboard = ({ ariaLabel = 'Click to copy', text, @@ -35,14 +41,14 @@ export const CopyToClipboard = ({ }) } - const transitions = useTransition(hasCopied, { - from: { opacity: 0, transform: 'scale(0.8)' }, - enter: { opacity: 1, transform: 'scale(1)' }, - leave: { opacity: 0, transform: 'scale(0.8)' }, - config: config.stiff, - trail: 100, - initial: null, - }) + const animateProps = { + className: 'absolute inset-0 flex items-center justify-center', + variants, + initial: 'hidden', + animate: 'visible', + exit: 'hidden', + transition: { type: 'spring', duration: 0.2, bounce: 0 }, + } return ( ) } diff --git a/app/ui/lib/DialogOverlay.tsx b/app/ui/lib/DialogOverlay.tsx index 005ebac86f..4fbe391925 100644 --- a/app/ui/lib/DialogOverlay.tsx +++ b/app/ui/lib/DialogOverlay.tsx @@ -6,12 +6,17 @@ * Copyright Oxide Computer Company */ +import * as m from 'motion/react-m' import { forwardRef } from 'react' export const DialogOverlay = forwardRef((_, ref) => ( -
)) diff --git a/app/ui/lib/Modal.tsx b/app/ui/lib/Modal.tsx index 261300f6ac..830a7fc089 100644 --- a/app/ui/lib/Modal.tsx +++ b/app/ui/lib/Modal.tsx @@ -6,8 +6,8 @@ * Copyright Oxide Computer Company */ import * as Dialog from '@radix-ui/react-dialog' -import { animated, useTransition } from '@react-spring/web' import cn from 'classnames' +import * as m from 'motion/react-m' import type { MergeExclusive } from 'type-fest' import { Close12Icon } from '@oxide/design-system/icons/react' @@ -41,63 +41,49 @@ export function Modal({ narrow, overlay = true, }: ModalProps) { - const AnimatedDialogContent = animated(Dialog.Content) - - const config = { tension: 650, mass: 0.125 } - - const transitions = useTransition(isOpen, { - from: { y: -5 }, - enter: { y: 0 }, - config: isOpen ? config : { duration: 0 }, - }) - return ( - {transitions( - ({ y }, item) => - item && ( - { - if (!open) onDismiss() - }} - // https://github.com/radix-ui/primitives/issues/1159#issuecomment-1559813266 - modal={false} + { + if (!open) onDismiss() + }} + modal={false} + > + + {overlay && } + e.preventDefault()} + aria-describedby={undefined} // radix warns without this + > + - - {overlay && } - - `translate3d(-50%, ${-50 + value}%, 0px)`), - }} - // Prevents cancel loop on clicking on background over side - // modal to get out of image upload modal. Canceling out of - // confirm dialog returns focus to the dismissable layer, - // which triggers onDismiss again. And again. - // https://github.com/oxidecomputer/console/issues/1745 - onFocusOutside={(e) => e.preventDefault()} - > - - {title} - - {children} - - - - - - - ) - )} + + {title} + + {children} + + + + + + + ) } diff --git a/app/ui/lib/SideModal.tsx b/app/ui/lib/SideModal.tsx index 01aae0f70c..5f1fe56603 100644 --- a/app/ui/lib/SideModal.tsx +++ b/app/ui/lib/SideModal.tsx @@ -6,8 +6,8 @@ * Copyright Oxide Computer Company */ import * as Dialog from '@radix-ui/react-dialog' -import { animated, useTransition } from '@react-spring/web' import cn from 'classnames' +import * as m from 'motion/react-m' import React, { useRef, type ReactNode } from 'react' import { Close12Icon, Error12Icon } from '@oxide/design-system/icons/react' @@ -49,85 +49,69 @@ export function SideModal({ onDismiss, title, subtitle, - isOpen, animate = true, errors, }: SideModalProps) { - const AnimatedDialogContent = animated(Dialog.Content) - - const config = { tension: 650, mass: 0.125 } - - const transitions = useTransition(isOpen, { - from: { x: 50 }, - enter: { x: 0 }, - config: isOpen && animate ? config : { duration: 0 }, - }) - return ( - {transitions( - ({ x }, item) => - item && ( - { - if (!open) onDismiss() - }} - // https://github.com/radix-ui/primitives/issues/1159#issuecomment-1559813266 - modal={false} + { + if (!open) onDismiss() + }} + // https://github.com/radix-ui/primitives/issues/1159#issuecomment-1559813266 + modal={false} + > + + + + - - - `translate3d(${value}%, 0px, 0px)`), - }} - // shuts off a warning from radix about dialog content needing a description - aria-describedby={undefined} - > -
- - {title} - - {subtitle} -
- {errors && errors.length > 0 && ( -
- -
{errors.length} issues:
-
    - {errors.map((error, idx) => ( -
  • {error}
    • - ))} -
- - ) - } - title={errors.length > 1 ? 'Errors' : 'Error'} - /> -
- )} - {children} - - {/* Close button is here at the end so we aren't automatically focusing on it when the side modal is opened. Positioned in the safe area at the top */} - - - - - - - ) - )} +
+ + {title} + + {subtitle} +
+ {errors && errors.length > 0 && ( +
+ +
{errors.length} issues:
+
    + {errors.map((error, idx) => ( +
  • {error}
    • + ))} +
+ + ) + } + title={errors.length > 1 ? 'Errors' : 'Error'} + /> +
+ )} + {children} + + {/* Close button is here at the end so we aren't automatically focusing on it when the side modal is opened. Positioned in the safe area at the top */} + + + + + + + ) } diff --git a/app/ui/lib/TimeoutIndicator.tsx b/app/ui/lib/TimeoutIndicator.tsx index ca19c20e2e..e2041982d9 100644 --- a/app/ui/lib/TimeoutIndicator.tsx +++ b/app/ui/lib/TimeoutIndicator.tsx @@ -5,38 +5,16 @@ * * Copyright Oxide Computer Company */ -import { animated, Globals, useTransition } from '@react-spring/web' -import cn from 'classnames' import { useTimeout } from './use-timeout' export interface TimeoutIndicatorProps { timeout: number onTimeoutEnd: () => void - className: string } -export const TimeoutIndicator = ({ - timeout, - onTimeoutEnd, - className, -}: TimeoutIndicatorProps) => { - const transitions = useTransition(true, { - from: { width: '0%' }, - enter: { width: '100%' }, - leave: { width: '100%' }, - config: { duration: timeout }, - }) - +export const TimeoutIndicator = ({ timeout, onTimeoutEnd }: TimeoutIndicatorProps) => { useTimeout(onTimeoutEnd, timeout) - // Don't show progress bar if reduce motion is turned on - if (Globals.skipAnimation) return null - - return transitions((styles) => ( - - )) + return null } diff --git a/app/ui/lib/Toast.tsx b/app/ui/lib/Toast.tsx index 62ea00a43a..8c0a727cd8 100644 --- a/app/ui/lib/Toast.tsx +++ b/app/ui/lib/Toast.tsx @@ -66,12 +66,6 @@ const secondaryTextColor: Record = { info: 'text-notice-secondary', } -const progressColor: Record = { - success: 'bg-accent-raise', - error: 'bg-destructive-raise', - info: 'bg-notice-raise', -} - export const Toast = ({ title, content, @@ -90,7 +84,7 @@ export const Toast = ({ return (
)}
- +
+ +
- {timeout !== null && ( - - )} + {timeout !== null && }
) } diff --git a/app/ui/styles/components/menu-button.css b/app/ui/styles/components/menu-button.css index 849902121c..b9f3617b19 100644 --- a/app/ui/styles/components/menu-button.css +++ b/app/ui/styles/components/menu-button.css @@ -45,7 +45,7 @@ .DropdownMenuContent, .DocsPopoverPanel { - animation: slide-down 0.2s ease; + animation: slide-down 0.2s var(--ease-out-quad); } @media (prefers-reduced-motion) { diff --git a/app/ui/styles/index.css b/app/ui/styles/index.css index 4862fece0a..52adedc6e7 100644 --- a/app/ui/styles/index.css +++ b/app/ui/styles/index.css @@ -45,6 +45,28 @@ :root { --content-gutter: 2.5rem; --top-bar-height: 54px; + + /* Nicer easing from: https://twitter.com/bdc */ + --ease-in-quad: cubic-bezier(0.55, 0.085, 0.68, 0.53); + --ease-in-cubic: cubic-bezier(0.55, 0.055, 0.675, 0.19); + --ease-in-quart: cubic-bezier(0.895, 0.03, 0.685, 0.22); + --ease-in-quint: cubic-bezier(0.755, 0.05, 0.855, 0.06); + --ease-in-expo: cubic-bezier(0.95, 0.05, 0.795, 0.035); + --ease-in-circ: cubic-bezier(0.6, 0.04, 0.98, 0.335); + + --ease-out-quad: cubic-bezier(0.25, 0.46, 0.45, 0.94); + --ease-out-cubic: cubic-bezier(0.215, 0.61, 0.355, 1); + --ease-out-quart: cubic-bezier(0.165, 0.84, 0.44, 1); + --ease-out-quint: cubic-bezier(0.23, 1, 0.32, 1); + --ease-out-expo: cubic-bezier(0.19, 1, 0.22, 1); + --ease-out-circ: cubic-bezier(0.075, 0.82, 0.165, 1); + + --ease-in-out-quad: cubic-bezier(0.455, 0.03, 0.515, 0.955); + --ease-in-out-cubic: cubic-bezier(0.645, 0.045, 0.355, 1); + --ease-in-out-quart: cubic-bezier(0.77, 0, 0.175, 1); + --ease-in-out-quint: cubic-bezier(0.86, 0, 0.07, 1); + --ease-in-out-expo: cubic-bezier(1, 0, 0, 1); + --ease-in-out-circ: cubic-bezier(0.785, 0.135, 0.15, 0.86); } @layer base { @@ -117,6 +139,22 @@ input[type='number']:focus-visible { } } +a, +button, +.ox-tabs-panel, +[role='listbox'], +[role='option'], +[role='button'], +input[type='text'], +input[type='textarea'], +textarea[type='text'], +input[type='file'], +input[type='radio'], +input[type='checkbox'], +input[type='number'] { + @apply transition-[outline-width] duration-100 ease-out; +} + a:focus, button:focus, .ox-tabs-panel:focus, diff --git a/app/util/motion-features.ts b/app/util/motion-features.ts new file mode 100644 index 0000000000..82b13deb9b --- /dev/null +++ b/app/util/motion-features.ts @@ -0,0 +1,10 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * Copyright Oxide Computer Company + */ + +// see https://motion.dev/docs/react-reduce-bundle-size#lazy-loading +export { domAnimation } from 'motion/react' diff --git a/package-lock.json b/package-lock.json index bd20cf4668..a4bf23bf05 100644 --- a/package-lock.json +++ b/package-lock.json @@ -18,7 +18,6 @@ "@radix-ui/react-focus-guards": "1.0.1", "@radix-ui/react-tabs": "^1.1.0", "@react-aria/live-announcer": "^3.3.4", - "@react-spring/web": "^9.7.4", "@tanstack/react-query": "^5.56.2", "@tanstack/react-query-devtools": "^5.56.2", "@tanstack/react-table": "^8.20.5", @@ -30,6 +29,7 @@ "lodash.throttle": "^4.1.1", "match-sorter": "^6.3.4", "md5": "^2.3.0", + "motion": "^11.16.2", "mousetrap": "^1.6.5", "p-map": "^7.0.2", "p-retry": "^6.2.0", @@ -3307,78 +3307,6 @@ "react": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0" } }, - "node_modules/@react-spring/animated": { - "version": "9.7.4", - "resolved": "https://registry.npmjs.org/@react-spring/animated/-/animated-9.7.4.tgz", - "integrity": "sha512-7As+8Pty2QlemJ9O5ecsuPKjmO0NKvmVkRR1n6mEotFgWar8FKuQt2xgxz3RTgxcccghpx1YdS1FCdElQNexmQ==", - "license": "MIT", - "dependencies": { - "@react-spring/shared": "~9.7.4", - "@react-spring/types": "~9.7.4" - }, - "peerDependencies": { - "react": "^16.8.0 || ^17.0.0 || ^18.0.0" - } - }, - "node_modules/@react-spring/core": { - "version": "9.7.4", - "resolved": "https://registry.npmjs.org/@react-spring/core/-/core-9.7.4.tgz", - "integrity": "sha512-GzjA44niEJBFUe9jN3zubRDDDP2E4tBlhNlSIkTChiNf9p4ZQlgXBg50qbXfSXHQPHak/ExYxwhipKVsQ/sUTw==", - "license": "MIT", - "dependencies": { - "@react-spring/animated": "~9.7.4", - "@react-spring/shared": "~9.7.4", - "@react-spring/types": "~9.7.4" - }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/react-spring/donate" - }, - "peerDependencies": { - "react": "^16.8.0 || ^17.0.0 || ^18.0.0" - } - }, - "node_modules/@react-spring/rafz": { - "version": "9.7.4", - "resolved": "https://registry.npmjs.org/@react-spring/rafz/-/rafz-9.7.4.tgz", - "integrity": "sha512-mqDI6rW0Ca8IdryOMiXRhMtVGiEGLIO89vIOyFQXRIwwIMX30HLya24g9z4olDvFyeDW3+kibiKwtZnA4xhldA==", - "license": "MIT" - }, - "node_modules/@react-spring/shared": { - "version": "9.7.4", - "resolved": "https://registry.npmjs.org/@react-spring/shared/-/shared-9.7.4.tgz", - "integrity": "sha512-bEPI7cQp94dOtCFSEYpxvLxj0+xQfB5r9Ru1h8OMycsIq7zFZon1G0sHrBLaLQIWeMCllc4tVDYRTLIRv70C8w==", - "license": "MIT", - "dependencies": { - "@react-spring/rafz": "~9.7.4", - "@react-spring/types": "~9.7.4" - }, - "peerDependencies": { - "react": "^16.8.0 || ^17.0.0 || ^18.0.0" - } - }, - "node_modules/@react-spring/types": { - "version": "9.7.4", - "resolved": "https://registry.npmjs.org/@react-spring/types/-/types-9.7.4.tgz", - "integrity": "sha512-iQVztO09ZVfsletMiY+DpT/JRiBntdsdJ4uqk3UJFhrhS8mIC9ZOZbmfGSRs/kdbNPQkVyzucceDicQ/3Mlj9g==", - "license": "MIT" - }, - "node_modules/@react-spring/web": { - "version": "9.7.4", - "resolved": "https://registry.npmjs.org/@react-spring/web/-/web-9.7.4.tgz", - "integrity": "sha512-UMvCZp7I5HCVIleSa4BwbNxynqvj+mJjG2m20VO2yPoi2pnCYANy58flvz9v/YcXTAvsmL655FV3pm5fbr6akA==", - "license": "MIT", - "dependencies": { - "@react-spring/animated": "~9.7.4", - "@react-spring/core": "~9.7.4", - "@react-spring/shared": "~9.7.4", - "@react-spring/types": "~9.7.4" - }, - "peerDependencies": { - "react": "^16.8.0 || ^17.0.0 || ^18.0.0", - "react-dom": "^16.8.0 || ^17.0.0 || ^18.0.0" - } - }, "node_modules/@react-stately/calendar": { "version": "3.5.4", "resolved": "https://registry.npmjs.org/@react-stately/calendar/-/calendar-3.5.4.tgz", @@ -8761,6 +8689,33 @@ "url": "https://github.com/sponsors/rawify" } }, + "node_modules/framer-motion": { + "version": "11.16.2", + "resolved": "https://registry.npmjs.org/framer-motion/-/framer-motion-11.16.2.tgz", + "integrity": "sha512-M946d8UhmI4lVZ4Wy2bLxw7D7LWw+OZTK5eCFCpGJNpUKt17oCP7+bBM3iKp6PfJF30ngBxsdxssFjLdD85ThA==", + "license": "MIT", + "dependencies": { + "motion-dom": "^11.16.1", + "motion-utils": "^11.16.0", + "tslib": "^2.4.0" + }, + "peerDependencies": { + "@emotion/is-prop-valid": "*", + "react": "^18.0.0 || ^19.0.0", + "react-dom": "^18.0.0 || ^19.0.0" + }, + "peerDependenciesMeta": { + "@emotion/is-prop-valid": { + "optional": true + }, + "react": { + "optional": true + }, + "react-dom": { + "optional": true + } + } + }, "node_modules/fresh": { "version": "0.5.2", "resolved": "https://registry.npmjs.org/fresh/-/fresh-0.5.2.tgz", @@ -10750,6 +10705,47 @@ "node": ">=16 || 14 >=14.17" } }, + "node_modules/motion": { + "version": "11.16.2", + "resolved": "https://registry.npmjs.org/motion/-/motion-11.16.2.tgz", + "integrity": "sha512-Q73vRcFCLTfKdIq8CllBi72zvntKEnaFaE3Wh0y0cWxeQUAw7VymVg8eZpLADZku7SNvk4GhZJqnIVl8eGepiw==", + "license": "MIT", + "dependencies": { + "framer-motion": "^11.16.2", + "tslib": "^2.4.0" + }, + "peerDependencies": { + "@emotion/is-prop-valid": "*", + "react": "^18.0.0 || ^19.0.0", + "react-dom": "^18.0.0 || ^19.0.0" + }, + "peerDependenciesMeta": { + "@emotion/is-prop-valid": { + "optional": true + }, + "react": { + "optional": true + }, + "react-dom": { + "optional": true + } + } + }, + "node_modules/motion-dom": { + "version": "11.16.1", + "resolved": "https://registry.npmjs.org/motion-dom/-/motion-dom-11.16.1.tgz", + "integrity": "sha512-XVNf3iCfZn9OHPZYJQy5YXXLn0NuPNvtT3YCat89oAnr4D88Cr52KqFgKa8dWElBK8uIoQhpJMJEG+dyniYycQ==", + "license": "MIT", + "dependencies": { + "motion-utils": "^11.16.0" + } + }, + "node_modules/motion-utils": { + "version": "11.16.0", + "resolved": "https://registry.npmjs.org/motion-utils/-/motion-utils-11.16.0.tgz", + "integrity": "sha512-ngdWPjg31rD4WGXFi0eZ00DQQqKKu04QExyv/ymlC+3k+WIgYVFbt6gS5JsFPbJODTF/r8XiE/X+SsoT9c0ocw==", + "license": "MIT" + }, "node_modules/mousetrap": { "version": "1.6.5", "resolved": "https://registry.npmjs.org/mousetrap/-/mousetrap-1.6.5.tgz", diff --git a/package.json b/package.json index 56f18247f7..14e7fc0ac0 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,6 @@ "@radix-ui/react-focus-guards": "1.0.1", "@radix-ui/react-tabs": "^1.1.0", "@react-aria/live-announcer": "^3.3.4", - "@react-spring/web": "^9.7.4", "@tanstack/react-query": "^5.56.2", "@tanstack/react-query-devtools": "^5.56.2", "@tanstack/react-table": "^8.20.5", @@ -52,6 +51,7 @@ "lodash.throttle": "^4.1.1", "match-sorter": "^6.3.4", "md5": "^2.3.0", + "motion": "^11.16.2", "mousetrap": "^1.6.5", "p-map": "^7.0.2", "p-retry": "^6.2.0", From a629300f4a0315d31f328008b86c5bfb4f16650c Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 10 Jan 2025 14:45:42 -0600 Subject: [PATCH 6/6] Full tests for row actions menu polling bug (#2648) tests for row actions menu polling bug --- .eslintrc.cjs | 11 +++++- test/e2e/instance-disks.e2e.ts | 44 ---------------------- test/e2e/instance.e2e.ts | 69 ++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 46 deletions(-) diff --git a/.eslintrc.cjs b/.eslintrc.cjs index fdea7f935f..85776837e5 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -114,11 +114,18 @@ module.exports = { }, { files: ['*.e2e.ts'], - extends: ['plugin:playwright/playwright-test'], + extends: ['plugin:playwright/recommended'], rules: { 'playwright/expect-expect': [ 'warn', - { assertFunctionNames: ['expectVisible', 'expectRowVisible', 'expectOptions'] }, + { + assertFunctionNames: [ + 'expectVisible', + 'expectRowVisible', + 'expectOptions', + 'expectRowMenuStaysOpen', + ], + }, ], 'playwright/no-force-option': 'off', }, diff --git a/test/e2e/instance-disks.e2e.ts b/test/e2e/instance-disks.e2e.ts index bd76faafcd..0c6a080102 100644 --- a/test/e2e/instance-disks.e2e.ts +++ b/test/e2e/instance-disks.e2e.ts @@ -7,14 +7,12 @@ */ import { clickRowAction, - closeToast, expect, expectNoToast, expectNotVisible, expectRowVisible, expectToast, expectVisible, - openRowActions, stopInstance, test, } from './utils' @@ -230,45 +228,3 @@ test('Change boot disk', async ({ page }) => { await expect(page.getByText('Attach a disk to be able to set a boot disk')).toBeVisible() }) - -// silly test but we've reintroduced this bug like 3 times -test("polling doesn't close row actions menu", async ({ page }) => { - await page.goto('/projects/mock-project/instances/db1') - - // stop, but don't wait until the state has changed - await page.getByRole('button', { name: 'Stop' }).click() - await page.getByRole('button', { name: 'Confirm' }).click() - await closeToast(page) - - const menu = page.getByRole('menu') - const stopped = page.getByText('statestopped') - - await expect(menu).toBeHidden() - await expect(stopped).toBeHidden() - - await openRowActions(page, 'disk-1') - await expect(stopped).toBeHidden() // still not stopped yet - await expect(menu).toBeVisible() - - // now we're stopped, which means polling has happened, but the - // menu remains visible - await expect(stopped).toBeVisible() - await expect(menu).toBeVisible() - - // now start it so we can check the non-boot disks table - await page.getByRole('button', { name: 'Start' }).click() - await page.getByRole('button', { name: 'Confirm' }).click() - await closeToast(page) - - const running = page.getByText('staterunning') // not running yet - await expect(running).toBeHidden() - await expect(menu).toBeHidden() - - await openRowActions(page, 'disk-2') - await expect(running).toBeHidden() // still not running yet - await expect(menu).toBeVisible() - - // state change means polling has happened. menu is still visible - await expect(running).toBeVisible() - await expect(menu).toBeVisible() -}) diff --git a/test/e2e/instance.e2e.ts b/test/e2e/instance.e2e.ts index 338e2392f2..2e6bab3b7c 100644 --- a/test/e2e/instance.e2e.ts +++ b/test/e2e/instance.e2e.ts @@ -7,6 +7,7 @@ */ import { clickRowAction, + closeToast, expect, expectRowVisible, openRowActions, @@ -240,3 +241,71 @@ test('instance table', async ({ page }) => { state: expect.stringMatching(/^starting\d+s$/), }) }) + +async function expectRowMenuStaysOpen(page: Page, rowSelector: string) { + // stop, but don't wait until the state has changed + await page.getByRole('button', { name: 'Stop' }).click() + await page.getByRole('button', { name: 'Confirm' }).click() + await closeToast(page) + + const menu = page.getByRole('menu') + const stopped = page.getByText('statestopped') + + await expect(menu).toBeHidden() + await expect(stopped).toBeHidden() + + await openRowActions(page, rowSelector) + await expect(stopped).toBeHidden() // still not stopped yet + await expect(menu).toBeVisible() + + // now we're stopped, which means polling has happened, but the + // menu remains visible + await expect(stopped).toBeVisible() + await expect(menu).toBeVisible() +} + +// silly tests, but we've reintroduced this bug like 3 times + +test("polling doesn't close row actions: IPs table", async ({ page }) => { + await page.goto('/projects/mock-project/instances/db1/networking') + await expectRowMenuStaysOpen(page, '123.4.56.0') +}) + +test("polling doesn't close row actions: NICs table", async ({ page }) => { + await page.goto('/projects/mock-project/instances/db1/networking') + await expectRowMenuStaysOpen(page, 'my-nic') +}) + +test("polling doesn't close row actions: boot disk", async ({ page }) => { + await page.goto('/projects/mock-project/instances/db1') + await expectRowMenuStaysOpen(page, 'disk-1') +}) + +test("polling doesn't close row actions: other disk", async ({ page }) => { + await page.goto('/projects/mock-project/instances/db1') + await expectRowMenuStaysOpen(page, 'disk-2') +}) + +test("polling doesn't close row actions: instances", async ({ page }) => { + await page.goto('/projects/mock-project/instances') + + // can't use the cool function because it's *slightly* different + await clickRowAction(page, 'db1', 'Stop') + await page.getByRole('button', { name: 'Confirm' }).click() + await closeToast(page) + + const menu = page.getByRole('menu') + const stopped = page.getByText('stopped') + + await expect(menu).toBeHidden() + await expect(stopped).toBeHidden() + + await openRowActions(page, 'db1') + await expect(stopped).toBeHidden() // still not stopped yet + await expect(menu).toBeVisible() + + // now we're stopped, which means polling has happened, but the + // menu remains visible + await expect(stopped).toBeVisible() + await expect(menu).toBeVisible() +})