Improve imports in PackageListing view #1580
Conversation
WalkthroughThis PR centralizes UI imports to the top-level @thunderstore/cyberstorm barrel, replacing deep subpath imports across multiple Remix app files. It adds and reorganizes public exports in packages/cyberstorm/src/index.ts, including New-prefixed components, utilities (e.g., formatToDisplayName), and SVG/logo exports. It introduces a shared SelectOption type in utils/types and updates Select and SelectSearch to consume it. useToast and various SVG/icon imports are switched to the barrel. packageListing.tsx updates types to use DapperTsInterface return types. Storybook stories align to NewSelectSearch and its types. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
Oksamies
force-pushed
the
10-03-replace_modal_components_implementation_with_radix_modal
branch
from
8ee7d26 to
ff7b4c4
Compare
anttimaki
force-pushed
the
packageListing-imports
branch
from
7da8d23 to
d3c3726
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx (1)
24-24: Inconsistent with PR objective: internal path still used.Line 24 imports
TableSortfrom an internal path (@thunderstore/cyberstorm/src/newComponents/Table/Table), which contradicts this PR's goal of using package exports rather than internal file paths. Consider exportingTableSortfrom the public API and updating this import accordingly.apps/cyberstorm-remix/app/upload/upload.tsx (1)
46-46: Avoid internal import for typing; use the DapperTs method type insteadImporting from
@thunderstore/dapper-ts/src/...ties you to internals purely for a type. Derive the type from the public API.Apply:
-import { postPackageSubmissionMetadata } from "@thunderstore/dapper-ts/src/methods/package"; @@ - type SubmitorOutput = Awaited< - ReturnType<typeof postPackageSubmissionMetadata> - >; + type SubmitorOutput = Awaited< + ReturnType<DapperTs["postPackageSubmissionMetadata"]> + >;Also applies to: 309-312
apps/cyberstorm-remix/app/p/packageListing.tsx (1)
284-287: Fix og:url composition; you’re stringifying an objectgetPublicEnvVariables returns an object. Using it directly in a template literal yields “[object Object]…”. Access the URL property.
- content={`${getPublicEnvVariables(["VITE_BETA_SITE_URL"])}${location.pathname}`} + content={`${getPublicEnvVariables(["VITE_BETA_SITE_URL"]).VITE_BETA_SITE_URL}${location.pathname}`}
🧹 Nitpick comments (15)
apps/cyberstorm-remix/app/settings/teams/team/tabs/Settings/Settings.tsx (1)
24-24: Consider consolidating this import for consistency.While outside the scope of this change, line 24 still uses a deep import path (
cyberstorm/utils/StrongForm/useStrongForm). For consistency with the PR's objective to use package exports rather than internal paths, consider updating this in a follow-up.apps/cyberstorm-remix/app/upload/upload.tsx (3)
48-48: Preferclassnamespackage (or a barrel re‑export) over internal utilsAvoid
@thunderstore/cyberstorm/src/...imports. Use the standardclassnameslib (or a public re-export if provided).-import { classnames } from "@thunderstore/cyberstorm/src/utils/utils"; +import cx from "classnames"; @@ - rootClasses={classnames( - "drag-n-drop", - file ? "drag-n-drop--success" : null - )} + rootClasses={cx("drag-n-drop", { "drag-n-drop--success": !!file })}Also applies to: 436-439
147-173: Hook deps: includetoastinuseCallback,startUploadinuseEffectPrevents stale closures and matches hooks rules.
- }, [file, requestConfig]); + }, [file, requestConfig, toast]); @@ - }, [file]); + }, [file, startUpload]);Also applies to: 174-178
458-471: Fix “Remove” click also opening file dialogStop the click from bubbling to the drop area; set button type to avoid implicit submit.
- <button + <button + type="button" className="drag-n-drop__remove-button" - onClick={() => { + onClick={(e) => { + e.preventDefault(); + e.stopPropagation(); // TODO: Clicking this causes the file select to open also setFile(null); if (fileInputRef.current) { fileInputRef.current.value = ""; } handle?.abort(); setHandle(undefined); setUsermedia(undefined); setIsDone(false); // setLock(false); }} >apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPageEdit.tsx (1)
34-34: Avoid deep internal import for classnames.This still reaches into a private path. Either re-export classnames from the cyberstorm barrel, or use the public alternative.
Apply in this file:
-import { classnames } from "@thunderstore/cyberstorm/src/utils/utils"; +import { classnames } from "@thunderstore/cyberstorm"And in packages/cyberstorm/src/index.ts (to support the above):
export { range, formatFileSize, formatInteger, formatToDisplayName, + classnames, } from "./utils/utils";apps/cyberstorm-remix/app/settings/teams/Teams.tsx (2)
24-25: Type alias uses return type from a different API; remove deep import and align.SubmitorOutput references postTeamCreate (deep import) while submitor uses teamCreate. Use teamCreate for the type and drop the deep import.
-import { postTeamCreate } from "@thunderstore/dapper-ts/src/methods/team"; @@ - type SubmitorOutput = Awaited<ReturnType<typeof postTeamCreate>>; + type SubmitorOutput = Awaited<ReturnType<typeof teamCreate>>;Also applies to: 142-146
27-31: Deep import from ts-api-react internals.SESSION_STORAGE_KEY should be consumed from that package’s public API to avoid internal coupling.
If not yet exported, consider re-exporting it from @thunderstore/ts-api-react and updating this import accordingly.
apps/cyberstorm-remix/app/settings/teams/team/tabs/Members/Members.tsx (1)
27-27: Replace deep TableSort import with public export.Use the barrel’s NewTableSort (or alias it locally).
-import { TableSort } from "@thunderstore/cyberstorm/src/newComponents/Table/Table"; +import { NewTableSort as TableSort } from "@thunderstore/cyberstorm"; @@ - sortDirection={TableSort.ASC} + sortDirection={TableSort.ASC}Also applies to: 181-185
packages/cyberstorm/src/index.ts (3)
99-101: Expose commonly used utils in the public API (classnames) to eliminate deep imports.Clients still import classnames from ./src/utils/utils. Re-export it here to complete the migration away from internal paths.
export { range, formatFileSize, formatInteger, formatToDisplayName, + classnames, } from "./utils/utils";Also applies to: 112-121
102-109: Consider also exporting TableSort without the New- alias for DX/back-compat.Many consumers expect TableSort; dual-export reduces churn.
export { Table as NewTable, TableSort as NewTableSort, type TableCompareColumnMeta, type TableRow as NewTableRow, type TableRows as NewTableRows, type TableLabels as NewTableLabels, } from "./newComponents/Table/Table"; +export { TableSort } from "./newComponents/Table/Table";
71-71: EmptyState namespace export may be awkward to consume.export * as EmptyState forces usage like EmptyState.EmptyState. Consider exporting the component directly for ergonomics.
-export * as EmptyState from "./newComponents/EmptyState"; +export { EmptyState } from "./newComponents/EmptyState/EmptyState";apps/cyberstorm-remix/app/p/tabs/Versions/Versions.tsx (1)
16-17: Minor: combine react-router imports.Reduce duplicate imports from the same module.
-import { Await, type LoaderFunctionArgs } from "react-router"; -import { useLoaderData } from "react-router"; +import { Await, useLoaderData, type LoaderFunctionArgs } from "react-router";apps/storybook/src/stories/cyberstormComponents/SelectSearch.stories.tsx (2)
97-101: Use SelectOption type instead of inline shape.Keeps types consistent with the component API.
-const [val, setVal] = useState<{ value: string; label?: string } | undefined>( - undefined -); +const [val, setVal] = useState<SelectOption | undefined>(undefined);
10-13: Avoid deep imports from theme package.Importing from @thunderstore/cyberstorm-theme/src/... couples to internals. Prefer the theme’s public API.
If available, switch to:
-import { - SelectSearchModifiersList, - SelectSearchSizesList, - SelectSearchVariantsList, -} from "@thunderstore/cyberstorm-theme/src/components"; +import { + SelectSearchModifiersList, + SelectSearchSizesList, + SelectSearchVariantsList, +} from "@thunderstore/cyberstorm-theme";If these aren’t exported yet, please add them to the theme barrel in this PR.
apps/cyberstorm-remix/app/p/packageListing.tsx (1)
68-68: Avoid “/types” deep import; use the package’s public type exportImporting CurrentUser from @thunderstore/dapper/types bypasses the package boundary.
- Prefer: import type { CurrentUser } from "@thunderstore/dapper" (or from "@thunderstore/dapper-ts" if that’s the intended surface).
- If not exported, re-export the type from the public entry.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
apps/cyberstorm-remix/app/commonComponents/Footer/Footer.tsx(1 hunks)apps/cyberstorm-remix/app/commonComponents/Navigation/Navigation.tsx(1 hunks)apps/cyberstorm-remix/app/p/packageEdit.tsx(1 hunks)apps/cyberstorm-remix/app/p/packageListing.tsx(8 hunks)apps/cyberstorm-remix/app/p/tabs/Versions/Versions.tsx(1 hunks)apps/cyberstorm-remix/app/p/tabs/Wiki/WikiNewPage.tsx(1 hunks)apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPageEdit.tsx(1 hunks)apps/cyberstorm-remix/app/settings/teams/Teams.tsx(1 hunks)apps/cyberstorm-remix/app/settings/teams/team/tabs/Members/Members.tsx(1 hunks)apps/cyberstorm-remix/app/settings/teams/team/tabs/Profile/Profile.tsx(1 hunks)apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx(1 hunks)apps/cyberstorm-remix/app/settings/teams/team/tabs/Settings/Settings.tsx(1 hunks)apps/cyberstorm-remix/app/settings/user/Account/Account.tsx(1 hunks)apps/cyberstorm-remix/app/settings/user/Connections/Connections.tsx(1 hunks)apps/cyberstorm-remix/app/upload/upload.tsx(1 hunks)apps/storybook/src/stories/cyberstormComponents/SelectSearch.stories.tsx(6 hunks)packages/cyberstorm-forms/src/useFormToaster.ts(1 hunks)packages/cyberstorm/src/index.ts(2 hunks)packages/cyberstorm/src/newComponents/Select/Select.tsx(1 hunks)packages/cyberstorm/src/newComponents/SelectSearch/SelectSearch.tsx(1 hunks)packages/cyberstorm/src/utils/types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/cyberstorm/src/utils/types.ts (1)
packages/cyberstorm/src/index.ts (1)
SelectOption(114-114)
apps/cyberstorm-remix/app/p/packageListing.tsx (1)
packages/dapper-ts/src/index.ts (1)
DapperTsInterface(31-34)
packages/cyberstorm/src/index.ts (1)
packages/cyberstorm/src/newComponents/EmptyState/EmptyState.tsx (1)
EmptyState(14-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Generate visual diffs
🔇 Additional comments (22)
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx (1)
11-11: LGTM! Import now uses public API.The
useToastimport correctly uses the package's public surface instead of an internal path.apps/cyberstorm-remix/app/settings/teams/team/tabs/Settings/Settings.tsx (1)
9-9: LGTM! Clean public API migration.The import consolidation correctly uses the barrel export from
@thunderstore/cyberstorm, aligning with the PR's objective to avoid internal path dependencies.apps/cyberstorm-remix/app/upload/upload.tsx (2)
12-13: Good move: useToast via public barrelThis aligns with the monorepo export policy and avoids internals.
35-35: Prefer public exports over internal paths
Check ifUserMediais publicly exported from the root of@thunderstore/ts-uploader; if so, change toimport { type UserMedia } from "@thunderstore/ts-uploader";otherwise, add a re-export in the package and then switch this import.
apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPageEdit.tsx (1)
21-22: Good move: useToast via public barrel.Importing from @thunderstore/cyberstorm matches the PR’s goal.
apps/cyberstorm-remix/app/settings/teams/team/tabs/Profile/Profile.tsx (1)
14-14: LGTM: useToast from public API.Aligned with the new export surface.
apps/cyberstorm-remix/app/settings/teams/Teams.tsx (1)
12-13: LGTM: useToast import standardized.Matches the PR objective.
apps/cyberstorm-remix/app/p/packageEdit.tsx (1)
9-11: LGTM: imports consolidated to barrel.formatToDisplayName and useToast from @thunderstore/cyberstorm are correct.
apps/cyberstorm-remix/app/settings/user/Account/Account.tsx (1)
8-9: LGTM: useToast via public export.No behavior change; cleaner dependency boundary.
apps/cyberstorm-remix/app/settings/teams/team/tabs/Members/Members.tsx (1)
13-16: LGTM: useToast and SelectOption from barrel.This removes internal path coupling.
packages/cyberstorm-forms/src/useFormToaster.ts (1)
1-1: LGTM!Clean import consolidation to the public API surface. The functionality remains unchanged.
apps/cyberstorm-remix/app/commonComponents/Footer/Footer.tsx (1)
10-11: LGTM!Import consolidation to the public API surface aligns with the PR objectives.
packages/cyberstorm/src/newComponents/Select/Select.tsx (1)
14-14: LGTM!Successfully migrated to the centralized
SelectOptiontype. This eliminates duplication betweenSelectandSelectSearchcomponents.apps/cyberstorm-remix/app/settings/user/Connections/Connections.tsx (1)
1-1: LGTM!Import consolidation to the public API surface. Both
OverwolfLogoanduseToastusage remain unchanged.apps/cyberstorm-remix/app/p/tabs/Wiki/WikiNewPage.tsx (1)
14-14: LGTM!Import consolidation aligns with the PR objectives. The
useToasthook usage remains unchanged.packages/cyberstorm/src/utils/types.ts (1)
1-7: LGTM! Successful type centralization.The new shared
SelectOptiontype eliminates duplication betweenSelectandSelectSearchcomponents. The type definition is properly exported and matches the previous local declarations.apps/cyberstorm-remix/app/commonComponents/Navigation/Navigation.tsx (1)
15-28: LGTM!Comprehensive import consolidation to the public API surface. All component usage remains unchanged throughout the file.
packages/cyberstorm/src/newComponents/SelectSearch/SelectSearch.tsx (1)
1-3: LGTM!Successfully migrated to the centralized
SelectOptiontype. The React import style change is appropriate sinceReactNodeis no longer needed in this file.apps/cyberstorm-remix/app/p/tabs/Versions/Versions.tsx (2)
6-15: Good move to barrel exports.Importing Heading, NewAlert, NewTableSort, ThunderstoreLogo from @thunderstore/cyberstorm reduces coupling to internals. LGTM.
5-15: Verify barrel exports ofversionsSchema,TableCompareColumnMeta, andTableRow. The grep search found no public exports—ensure they’re exposed from@thunderstore/dapper-tsand@thunderstore/cyberstormroots or update the imports accordingly.apps/storybook/src/stories/cyberstormComponents/SelectSearch.stories.tsx (1)
3-7: Nice consolidation to public exports.Switching NewSelectSearch, NewSelectSearchProps, and SelectOption to the barrel aligns stories with the package API. LGTM.
apps/cyberstorm-remix/app/p/packageListing.tsx (1)
942-945: Good move: aligning helper/prop types to DapperTsInterfaceUsing Awaited<ReturnType<DapperTsInterface["…"]>> across helpers and props removes typeof dependencies on local functions and matches the public surface.
Also applies to: 964-966, 1018-1019, 1109-1111, 1189-1190
There seemed to be some system where components were grouped by their import path, but then that system wasn't actually followed in all cases. In addition to fixing this issue, sort the exports alphabetically by their paths within the group.
Both Select and SelectSearch defined SelectOption, but only one of them exported it. Perhaps this was an attempt to avoid naming conflict, but in practice it forces @thunderstorm/cyberstorm-remix to either import using a direct path (breaking the separation of packages) or use the SelectOption defined in the sister component. Neither is ideal, so the type is now defined once in a helper file and imported/exported from there for everyone to use.
Ideally @thunderstore/cyberstorm would only export the components and tightly coupled things. If these helper functions are used by multiple packages in the repo, they should probably reside in their own neat package. But just exporting them properly is better than nothing.
anttimaki
force-pushed
the
packageListing-imports
branch
from
d3c3726 to
3b7b553
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
apps/cyberstorm-remix/app/commonComponents/Footer/Footer.tsx(1 hunks)apps/cyberstorm-remix/app/commonComponents/Navigation/Navigation.tsx(1 hunks)apps/cyberstorm-remix/app/p/packageEdit.tsx(1 hunks)apps/cyberstorm-remix/app/p/packageListing.tsx(8 hunks)apps/cyberstorm-remix/app/p/tabs/Wiki/WikiNewPage.tsx(1 hunks)apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPageEdit.tsx(1 hunks)apps/cyberstorm-remix/app/settings/teams/Teams.tsx(1 hunks)apps/cyberstorm-remix/app/settings/teams/team/tabs/Members/Members.tsx(1 hunks)apps/cyberstorm-remix/app/settings/teams/team/tabs/Profile/Profile.tsx(1 hunks)apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx(1 hunks)apps/cyberstorm-remix/app/settings/teams/team/tabs/Settings/Settings.tsx(1 hunks)apps/cyberstorm-remix/app/settings/user/Account/Account.tsx(1 hunks)apps/cyberstorm-remix/app/settings/user/Connections/Connections.tsx(1 hunks)apps/cyberstorm-remix/app/upload/upload.tsx(1 hunks)apps/storybook/src/stories/cyberstormComponents/SelectSearch.stories.tsx(6 hunks)packages/cyberstorm-forms/src/useFormToaster.ts(1 hunks)packages/cyberstorm/src/index.ts(2 hunks)packages/cyberstorm/src/newComponents/Select/Select.tsx(1 hunks)packages/cyberstorm/src/newComponents/SelectSearch/SelectSearch.tsx(1 hunks)packages/cyberstorm/src/utils/types.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/cyberstorm-remix/app/p/tabs/Wiki/WikiNewPage.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/cyberstorm/src/newComponents/Select/Select.tsx
- apps/cyberstorm-remix/app/settings/user/Connections/Connections.tsx
- apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx
- apps/cyberstorm-remix/app/settings/teams/team/tabs/Profile/Profile.tsx
- apps/cyberstorm-remix/app/upload/upload.tsx
- apps/cyberstorm-remix/app/settings/teams/team/tabs/Settings/Settings.tsx
- apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPageEdit.tsx
- apps/cyberstorm-remix/app/settings/teams/Teams.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
packages/cyberstorm/src/utils/types.ts (1)
packages/cyberstorm/src/index.ts (1)
SelectOption(114-114)
packages/cyberstorm/src/index.ts (1)
packages/cyberstorm/src/newComponents/EmptyState/EmptyState.tsx (1)
EmptyState(14-18)
apps/cyberstorm-remix/app/p/packageListing.tsx (1)
packages/dapper-ts/src/index.ts (1)
DapperTsInterface(33-36)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build
- GitHub Check: Generate visual diffs
🔇 Additional comments (12)
packages/cyberstorm/src/utils/types.ts (1)
1-7: LGTM! Clean centralization of the SelectOption type.The generic type with sensible defaults is well-designed. Centralizing this type in utils/types.ts and exporting it from the public API surface is exactly the right pattern for a monorepo package.
apps/cyberstorm-remix/app/settings/user/Account/Account.tsx (1)
8-8: LGTM! Proper use of barrel export.The migration from internal path to the public API surface is consistent with the PR objective.
apps/cyberstorm-remix/app/commonComponents/Footer/Footer.tsx (1)
10-11: LGTM! Clean SVG export consolidation.The ThunderstoreLogoHorizontal import now uses the public API surface as intended.
apps/cyberstorm-remix/app/commonComponents/Navigation/Navigation.tsx (1)
17-28: LGTM! Comprehensive import consolidation.All UI components, utilities, and SVG exports now properly use the public barrel surface. The addition of Modal and alignment with New-prefixed exports is consistent with the broader refactor.
apps/cyberstorm-remix/app/settings/teams/team/tabs/Members/Members.tsx (1)
13-14: LGTM! Type and hook imports properly consolidated.Both the SelectOption type and useToast hook now use the public barrel export, maintaining consistency with the broader import refactor.
apps/cyberstorm-remix/app/p/packageEdit.tsx (1)
9-10: LGTM! Utility and hook imports consolidated.Both formatToDisplayName and useToast now properly use the public barrel export.
packages/cyberstorm/src/newComponents/SelectSearch/SelectSearch.tsx (1)
1-3: LGTM! SelectOption type properly centralized.The component now imports SelectOption from the shared utils/types module instead of declaring it locally. The simplified React import is appropriate since ReactNode is now part of the external type definition.
apps/storybook/src/stories/cyberstormComponents/SelectSearch.stories.tsx (2)
3-7: LGTM! Storybook aligned with public API.The stories now use NewSelectSearch, NewSelectSearchProps, and SelectOption from the public barrel export, consistent with the broader refactor.
51-51: LGTM! Component props updated to use public types.All story components now use NewSelectSearchProps instead of the internal SelectSearchProps, properly aligning with the public API surface.
Also applies to: 78-78, 97-97, 134-134, 165-165
apps/cyberstorm-remix/app/p/packageListing.tsx (2)
45-65: LGTM! Imports successfully consolidated to public APIThe imports from
@thunderstore/cyberstormnow use the public barrel export, bringing in components, types, and utilities without reaching into internal paths. This aligns perfectly with the PR objective.
943-944: LGTM! Type signatures properly aligned with DapperTsInterfaceThe function signatures now reference
DapperTsInterfacereturn types rather than direct function types. This decouples the code from implementation details and relies on the interface contract, which is a cleaner architectural approach.Also applies to: 964-965, 1018-1018, 1109-1110, 1189-1189
packages/cyberstorm/src/index.ts (1)
29-126: LGTM! Public API properly expandedThe barrel export now surfaces:
- New-prefixed component aliases (NewAlert, NewButton, NewLink, etc.)
- Shared types like
SelectOptionfor consistency across consumers- Utility functions (formatFileSize, formatInteger, formatToDisplayName, range)
- SVG logo components
This consolidation enables consuming apps to import from the package root rather than reaching into internal paths, exactly as the PR intends.
There was a problem hiding this comment.
All good changes here! I've been meaning to add something like eslints import sorting, but haven't gotten around it https://eslint.org/docs/latest/rules/sort-imports
2 participants
The packages in this mono repo should export anything that using them requires. The app shouldn't do imports using exact file paths even though it's technically possible due to monorepo, as the paths are internal implementation details of each package.