[WIP]chore: normalize import order across codebase#2231
[WIP]chore: normalize import order across codebase#2231RYGRIT wants to merge 1 commit intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
RYGRIT
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThis pull request reorganises import statements across the entire codebase without introducing any functional logic changes, runtime behaviour modifications, or exported API alterations. The changes include reordering type-only imports to appear at the top of files, consolidating duplicate imports into single statements, grouping imports by source or category, and removing unnecessary blank lines between import blocks. No component behaviour, control flow, or business logic is affected. Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
test/unit/app/utils/versions.spec.ts (1)
18-20: Avoid non-null assertion ontags[0]in helper return
primaryTag: tags[0]!bypasses type safety and can mask an empty-tags bug in tests. Prefer a guarded fallback (or throw explicitly) to keep index access safe.Suggested refactor
function row(version: string, tags: string[]) { - return { id: `version:${version}`, primaryTag: tags[0]!, tags, version } + const primaryTag = tags[0] ?? '' + return { id: `version:${version}`, primaryTag, tags, version } }As per coding guidelines, “Ensure you write strictly type-safe code, for example by ensuring you always check when accessing an array value by index”.
scripts/unocss-checker.ts (1)
5-6: Consider consolidating imports from the same package.Both lines import from
unocssand could be combined into a single import statement for conciseness.♻️ Consolidate imports
-import { createGenerator } from 'unocss' -import { presetWind4 } from 'unocss' +import { createGenerator, presetWind4 } from 'unocss'server/api/auth/atproto.get.ts (1)
4-5: Consider consolidating imports from the same module.
isAtUriStringandisAtIdentifierStringare imported separately from@atproto/lexon consecutive lines. They could be combined into a single import statement for consistency with the rest of the file.♻️ Suggested change
-import { Client, isAtUriString } from '@atproto/lex' -import { isAtIdentifierString } from '@atproto/lex' +import { Client, isAtIdentifierString, isAtUriString } from '@atproto/lex'modules/blog.ts (1)
4-13: Import grouping follows the stated convention.The reorganization correctly groups external imports (lines 4–12) before relative imports (line 13 onwards), adhering to the project's type → external → internal → relative ordering.
For enhanced readability, consider one of these optional refinements within the external import group:
- Option 1: Alphabetize all external imports together
- Option 2: Subgroup by category (e.g., regular packages, then node built-ins, then framework packages)
- Option 3: Add a blank line between external and relative import groups
♻️ Example: Subgrouped and separated
import shiki from '@shikijs/markdown-exit' import { defu } from 'defu' import { read } from 'gray-matter' import MarkdownItAnchor from 'markdown-it-anchor' +import { globSync } from 'tinyglobby' +import Markdown from 'unplugin-vue-markdown/vite' +import { array, safeParse } from 'valibot' + import crypto from 'node:crypto' import { existsSync } from 'node:fs' import { mkdir, writeFile } from 'node:fs/promises' import { join } from 'node:path' + import { addTemplate, addVitePlugin, defineNuxtModule, useNuxt, createResolver } from 'nuxt/kit' -import { globSync } from 'tinyglobby' -import Markdown from 'unplugin-vue-markdown/vite' -import { array, safeParse } from 'valibot' + import { isProduction } from '../config/env'
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bb500ae7-f4bf-406c-953c-843b9080119a
📒 Files selected for processing (96)
.storybook/manager.ts.storybook/preview.tsapp/components/AppHeader.vueapp/components/BlueskyComment.vueapp/components/Compare/FacetBarChart.vueapp/components/Header/AuthModal.client.vueapp/components/Header/MobileMenu.client.vueapp/components/Package/InstallScripts.vueapp/components/Package/TrendsChart.vueapp/components/Package/Versions.vueapp/components/Package/WeeklyDownloadStats.vueapp/composables/useGlobalSearch.tsapp/composables/useSettings.tsapp/composables/useVersionDistribution.tsapp/pages/about.stories.tsapp/pages/compare.vueapp/pages/org/[org].vueapp/pages/search.vueapp/utils/atproto/likes.tsapp/utils/run-command.tscli/src/mock-app.tscli/src/server.tsconfig/env.tslunaria.config.tslunaria/components.tslunaria/lunaria.tsmodules/blog.tsmodules/runtime/server/cache.tsmodules/standard-site-sync.tsnuxt.config.tsplaywright.config.tsscripts/remove-unused-translations.tsscripts/unocss-checker.tsserver/api/atproto/bluesky-author-profiles.get.tsserver/api/atproto/bluesky-comments.get.tsserver/api/atproto/pds-graphs.get.tsserver/api/atproto/pds-users.get.tsserver/api/auth/atproto.get.tsserver/api/auth/session.get.tsserver/api/jsr/[...pkg].get.tsserver/api/registry/analysis/[...pkg].get.tsserver/api/registry/file/[...pkg].get.tsserver/api/registry/org/[org]/packages.get.tsserver/api/social/like.post.tsserver/api/social/profile/[identifier]/index.put.tsserver/routes/skills/[...pkg].get.tsserver/utils/atproto/oauth.tsserver/utils/atproto/utils/likes.tsserver/utils/atproto/utils/profile.tsserver/utils/dependency-analysis.tsserver/utils/dependency-resolver.tsserver/utils/docs/client.tsserver/utils/docs/render.tsserver/utils/docs/text.tsserver/utils/error-handler.tsserver/utils/readme.tsshared/schemas/blog.tsshared/utils/atproto.tsshared/utils/constellation.tsshared/utils/diff.tstest/nuxt/a11y.spec.tstest/nuxt/components/HeaderConnectorModal.spec.tstest/nuxt/components/OgImagePackage.spec.tstest/nuxt/components/Package/LikeCard.spec.tstest/nuxt/components/Package/MetricsBadges.spec.tstest/nuxt/components/Package/Sidebar.spec.tstest/nuxt/components/Package/Versions.spec.tstest/nuxt/components/Package/WeeklyDownloadStats.spec.tstest/nuxt/components/ProfileInviteSection.spec.tstest/nuxt/components/compare/FacetSelector.spec.tstest/nuxt/composables/use-colors.spec.tstest/nuxt/composables/use-compare-replacements.spec.tstest/nuxt/composables/use-facet-selection.spec.tstest/nuxt/composables/use-install-command.spec.tstest/nuxt/composables/use-package-comparison.spec.tstest/nuxt/composables/use-replacement-dependencies.spec.tstest/nuxt/pages/PackageVersionsPage.spec.tstest/unit/app/composables/use-charts.spec.tstest/unit/app/utils/chart-data-prediction.spec.tstest/unit/app/utils/charts.spec.tstest/unit/app/utils/download-anomalies.spec.tstest/unit/app/utils/install-command.spec.tstest/unit/app/utils/npm/common.spec.tstest/unit/app/utils/run-command.spec.tstest/unit/app/utils/versions.spec.tstest/unit/server/utils/dependency-resolver.spec.tstest/unit/server/utils/docs/format.spec.tstest/unit/server/utils/docs/render.spec.tstest/unit/server/utils/docs/text.spec.tstest/unit/server/utils/file-tree.spec.tstest/unit/server/utils/import-resolver.spec.tstest/unit/server/utils/likes-evolution.spec.tstest/unit/shared/types/index.spec.tstest/unit/uno-preset-a11y.spec.tstest/unit/uno-preset-rtl.spec.tsuno.config.ts
💤 Files with no reviewable changes (3)
- .storybook/manager.ts
- server/routes/skills/[...pkg].get.ts
- test/unit/app/utils/npm/common.spec.ts
| @@ -1,36 +1,9 @@ | |||
| import crypto from 'node:crypto' | |||
| import { H3, HTTPError, handleCors, type H3Event } from 'h3-next' | |||
| import type { ConnectorState, PendingOperation, ApiResponse, ConnectorEndpoints } from './types.ts' | |||
There was a problem hiding this comment.
Reinstate compile-time endpoint parity check.
Line 1 now omits the AssertEndpointsImplemented safeguard, so endpoint drift between ConnectorEndpoints and this server can go unnoticed at compile time.
Suggested fix
-import type { ConnectorState, PendingOperation, ApiResponse, ConnectorEndpoints } from './types.ts'
+import type {
+ ConnectorState,
+ PendingOperation,
+ ApiResponse,
+ ConnectorEndpoints,
+ AssertEndpointsImplemented,
+} from './types.ts' export const CONNECTOR_VERSION = pkg.version
+
+type ImplementedEndpoint =
+ | 'POST /connect'
+ | 'GET /state'
+ | 'POST /operations'
+ | 'POST /operations/batch'
+ | 'DELETE /operations'
+ | 'DELETE /operations/all'
+ | 'POST /approve'
+ | 'POST /approve-all'
+ | 'POST /retry'
+ | 'POST /execute'
+ | 'GET /org/:org/users'
+ | 'GET /org/:org/teams'
+ | 'GET /team/:scopeTeam/users'
+ | 'GET /package/:pkg/collaborators'
+ | 'GET /user/packages'
+ | 'GET /user/orgs'
+
+const _endpointCheck: AssertEndpointsImplemented<ImplementedEndpoint> = true| import type { H3Event } from 'h3' | ||
| import { SLINGSHOT_HOST } from '#shared/utils/constants' | ||
| import { useServerSession } from '#server/utils/server-session' | ||
| import { clientUri } from '#oauth/config' |
There was a problem hiding this comment.
Restore @ts-expect-error for virtual module import.
Same issue as in server/utils/atproto/oauth.ts: the #oauth/config module is dynamically generated at build time and lacks TypeScript declarations. The removed @ts-expect-error suppression is causing the type check to fail.
🐛 Proposed fix
import { createError, getQuery, sendRedirect, setCookie, getCookie, deleteCookie } from 'h3'
+// `@ts-expect-error` virtual file from oauth module
import { clientUri } from '#oauth/config'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { clientUri } from '#oauth/config' | |
| // `@ts-expect-error` virtual file from oauth module | |
| import { clientUri } from '#oauth/config' |
🧰 Tools
🪛 GitHub Check: 💪 Type check
[failure] 8-8:
Cannot find module '#oauth/config' or its corresponding type declarations.
[failure] 8-8:
Cannot find module '#oauth/config' or its corresponding type declarations.
| import type { EventHandlerRequest, H3Event, SessionManager } from 'h3' | ||
| import { JoseKey, Keyset, oauthRedirectUriSchema, webUriSchema } from '@atproto/oauth-client-node' | ||
| import { NodeOAuthClient, AtprotoDohHandleResolver } from '@atproto/oauth-client-node' | ||
| import { clientUri } from '#oauth/config' |
There was a problem hiding this comment.
Restore @ts-expect-error for virtual module import.
The #oauth/config module is generated at build time via addServerTemplate in modules/oauth.ts and has no static TypeScript declarations. The @ts-expect-error comment that was removed was intentionally suppressing TS2307. Its removal is causing the type check pipeline to fail.
🐛 Proposed fix
import { JoseKey, Keyset, oauthRedirectUriSchema, webUriSchema } from '@atproto/oauth-client-node'
import { NodeOAuthClient, AtprotoDohHandleResolver } from '@atproto/oauth-client-node'
+// `@ts-expect-error` virtual file from oauth module
import { clientUri } from '#oauth/config'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { clientUri } from '#oauth/config' | |
| // `@ts-expect-error` virtual file from oauth module | |
| import { clientUri } from '#oauth/config' |
🧰 Tools
🪛 GitHub Check: 💪 Type check
[failure] 11-11:
Cannot find module '#oauth/config' or its corresponding type declarations.
[failure] 11-11:
Cannot find module '#oauth/config' or its corresponding type declarations.
| import type { ReplacementSuggestion } from '~/composables/useCompareReplacements' | ||
| import type { ModuleReplacement } from 'module-replacements' |
There was a problem hiding this comment.
Import order doesn't follow the stated convention.
According to the PR objectives, imports should follow: "type → external → internal → relative". The external type import on Line 2 (module-replacements) should come before the internal type import on Line 1 (~/composables/useCompareReplacements).
🔄 Proposed fix to correct import order
-import type { ReplacementSuggestion } from '~/composables/useCompareReplacements'
import type { ModuleReplacement } from 'module-replacements'
+import type { ReplacementSuggestion } from '~/composables/useCompareReplacements'
import { mountSuspended } from '@nuxt/test-utils/runtime'
import { afterEach, describe, expect, it, vi } from 'vitest'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import type { ReplacementSuggestion } from '~/composables/useCompareReplacements' | |
| import type { ModuleReplacement } from 'module-replacements' | |
| import type { ModuleReplacement } from 'module-replacements' | |
| import type { ReplacementSuggestion } from '~/composables/useCompareReplacements' | |
| import { mountSuspended } from '@nuxt/test-utils/runtime' | |
| import { afterEach, describe, expect, it, vi } from 'vitest' |
ghostdevv
left a comment
ghostdevv
left a comment
There was a problem hiding this comment.
are we doing this programmatically?
2 participants
🔗 Linked issue
🧭 Context
📚 Description