refactor: page store hooks #6409
Conversation
WalkthroughThis pull request introduces a comprehensive refactoring of page store management in the application. The primary change involves introducing an enum Changes
Sequence DiagramsequenceDiagram
participant Component as React Component
participant PageStore as usePageStore
participant StoreContext as Store Context
Component->>PageStore: Call with storeType
PageStore->>StoreContext: Retrieve context
StoreContext-->>PageStore: Return store data
PageStore-->>Component: Return page data
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (1)
web/core/components/pages/list/block.tsx (1)
48-48: Consider using React Context for store type.While the current implementation works well, passing
storeTypethrough props could lead to props drilling as the component tree grows. Consider using React Context if this pattern becomes common across many components.+ import { createContext, useContext } from 'react'; + + const PageStoreContext = createContext<EPageStoreType | undefined>(undefined); + + export const PageStoreProvider: FC<{ storeType: EPageStoreType; children: ReactNode }> = ({ + storeType, + children, + }) => ( + <PageStoreContext.Provider value={storeType}> + {children} + </PageStoreContext.Provider> + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(list)/page.tsx(2 hunks)web/ce/hooks/store/index.ts(1 hunks)web/ce/hooks/store/use-page-store.ts(1 hunks)web/ce/hooks/store/use-page.ts(1 hunks)web/core/components/pages/dropdowns/actions.tsx(3 hunks)web/core/components/pages/list/block-item-action.tsx(2 hunks)web/core/components/pages/list/block.tsx(2 hunks)web/core/components/pages/list/root.tsx(1 hunks)web/core/components/pages/modals/delete-page-modal.tsx(1 hunks)web/core/hooks/store/index.ts(0 hunks)web/core/hooks/store/pages/index.ts(0 hunks)web/core/hooks/store/pages/use-page.ts(0 hunks)web/core/store/pages/base-page.ts(0 hunks)web/ee/hooks/store/index.ts(1 hunks)
💤 Files with no reviewable changes (4)
- web/core/hooks/store/index.ts
- web/core/hooks/store/pages/index.ts
- web/core/store/pages/base-page.ts
- web/core/hooks/store/pages/use-page.ts
✅ Files skipped from review due to trivial changes (2)
- web/ee/hooks/store/index.ts
- web/ce/hooks/store/index.ts
🔇 Additional comments (13)
web/ce/hooks/store/use-page-store.ts (1)
7-13: LGTM! Well-structured type definitions.The enum and return type mapping provide a good foundation for extending to other page store types in the future.
web/ce/hooks/store/use-page.ts (1)
9-12: LGTM! Clear type definition.The
TArgstype clearly defines the required parameters for the hook.web/core/components/pages/list/root.tsx (1)
7-28: LGTM! Clean integration of the new store type system.The changes correctly integrate the new
storeTypeprop andusePageStorehook while maintaining type safety.web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(list)/page.tsx (1)
15-16: LGTM! Correct usage of the new store type system.The integration of
EPageStoreType.PROJECTis appropriate for this project-specific page component.Also applies to: 56-56
web/core/components/pages/modals/delete-page-modal.tsx (3)
10-13: LGTM! Clean transition to generic store hooks.The change from specific
useProjectPagesto genericusePageStorewithEPageStoreTypeimproves modularity while maintaining type safety.
24-24: LGTM! Consistent store access pattern.The change to use
usePageStore(storeType)maintains the same functionality while making the store type configurable.Also applies to: 28-28
19-20: Verify the impact of the new storeType prop.The addition of the required
storeTypeprop changes the component's contract. Ensure all existing usages ofDeletePageModalare updated to provide this prop.✅ Verification successful
All usages of
DeletePageModalare properly updated with the newstoreTypepropThe only usage of this component in
web/core/components/pages/dropdowns/actions.tsxalready includes the requiredstoreTypeprop. No further updates are needed.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all DeletePageModal usages to verify prop updates rg "DeletePageModal" -A 5Length of output: 2836
web/core/components/pages/dropdowns/actions.tsx (2)
33-33: LGTM! Clean type definition update.The addition of
storeTypeto Props maintains type safety while supporting the generic store pattern.Also applies to: 58-58
179-184: LGTM! Proper prop forwarding.The
storeTypeprop is correctly passed to theDeletePageModal, maintaining the component contract.web/core/components/pages/list/block-item-action.tsx (2)
16-24: LGTM! Consistent type propagation.The addition of
storeTypeto Props maintains consistency with the new store pattern.
87-87: LGTM! Proper prop forwarding.The
storeTypeprop is correctly passed toPageActions, completing the prop chain.web/core/components/pages/list/block.tsx (2)
14-15: LGTM! Clean implementation of store type management.The introduction of
EPageStoreTypeenum and the new hook imports provide a type-safe approach to store management, aligning well with the refactoring objectives.Also applies to: 19-19
23-23: Verify hook usage across the codebase.The implementation of
usePagehook with the new store type parameter looks good. However, let's verify that this pattern is consistently followed across other components.Also applies to: 27-30
✅ Verification successful
The hook implementation is consistent across the codebase
The
usePagehook is used only in the reviewed component and follows the correct pattern with bothpageIdandstoreTypeparameters. No inconsistencies or legacy implementations were found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all usages of usePage hook to ensure consistent implementation ast-grep --pattern 'usePage({ $$$, storeType: $_ })' # Also check for any potential missed migrations rg "usePage\(\s*{?\s*pageId" -A 2Length of output: 209
Script:
#!/bin/bash # Search for usePage hook usages with different patterns ast-grep --pattern 'usePage($$$)' # Search with ripgrep for all usePage occurrences rg "usePage" -A 3 --type ts --type tsx # Specific search for potential old pattern usage rg "usePage\s*\(\s*\{?\s*pageId" -A 3 --type ts --type tsxLength of output: 425
Script:
#!/bin/bash # Search for all usePage occurrences rg -i "usePage" -A 3 ".*\.(ts|tsx)$" # Look for potential old pattern usage rg -i "usePage\s*\(\s*\{?\s*pageId" -A 3 ".*\.(ts|tsx)$" # Find any imports of usePage to trace usage rg "import.*usePage" ".*\.(ts|tsx)$"Length of output: 296
Script:
#!/bin/bash # Search for all usePage occurrences rg -i "usePage" -g "*.ts" -g "*.tsx" -A 3 # Find any imports of usePage rg "import.*usePage" -g "*.ts" -g "*.tsx" # Look for any usePage function calls rg "usePage\(" -g "*.ts" -g "*.tsx" -A 3Length of output: 13097
web/ce/hooks/store/use-page-store.ts
Outdated
| export const usePageStore = <T extends EPageStoreType>(storeType: T): TReturnType[T] => { | ||
| const context = useContext(StoreContext); | ||
| if (context === undefined) throw new Error("useProjectPage must be used within StoreProvider"); | ||
|
|
||
| if (storeType === EPageStoreType.PROJECT) { | ||
| return context.projectPages; | ||
| } | ||
|
|
||
| return context.projectPages; | ||
| }; |
There was a problem hiding this comment.
Fix type safety in the default return path.
The default return of context.projectPages outside the if block makes the generic type unsafe. This could cause runtime errors when more store types are added in the future.
Apply this diff to ensure type safety:
export const usePageStore = <T extends EPageStoreType>(storeType: T): TReturnType[T] => {
const context = useContext(StoreContext);
if (context === undefined) throw new Error("useProjectPage must be used within StoreProvider");
if (storeType === EPageStoreType.PROJECT) {
return context.projectPages;
}
- return context.projectPages;
+ throw new Error(`Invalid store type: ${storeType}`);
};📝 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.
| export const usePageStore = <T extends EPageStoreType>(storeType: T): TReturnType[T] => { | |
| const context = useContext(StoreContext); | |
| if (context === undefined) throw new Error("useProjectPage must be used within StoreProvider"); | |
| if (storeType === EPageStoreType.PROJECT) { | |
| return context.projectPages; | |
| } | |
| return context.projectPages; | |
| }; | |
| export const usePageStore = <T extends EPageStoreType>(storeType: T): TReturnType[T] => { | |
| const context = useContext(StoreContext); | |
| if (context === undefined) throw new Error("useProjectPage must be used within StoreProvider"); | |
| if (storeType === EPageStoreType.PROJECT) { | |
| return context.projectPages; | |
| } | |
| throw new Error(`Invalid store type: ${storeType}`); | |
| }; |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
web/core/components/pages/modals/delete-page-modal.tsx (1)
19-20: LGTM! Consider using a const assertion for better type safety.The migration to the generic page store hook is clean. Consider using a const assertion for
EPageStoreTypeto make it more type-safe:-type TConfirmPageDeletionProps = { +type TConfirmPageDeletionProps = { isOpen: boolean; onClose: () => void; page: TPageInstance; - storeType: EPageStoreType; + storeType: EPageStoreType & {}; // const assertion for better type safety };Also applies to: 24-24, 28-28
web/core/components/pages/dropdowns/actions.tsx (1)
58-58: Consider grouping related props for better maintainability.The changes look good, but we could improve the props organization. Consider grouping related props using a discriminated union:
type Props = { editorRef?: EditorRefApi | null; extraOptions?: (TContextMenuItem & { key: TPageActions })[]; optionsOrder: TPageActions[]; page: TPageInstance; parentRef?: React.RefObject<HTMLElement>; - storeType: EPageStoreType; + store: { + type: EPageStoreType; + }; };This grouping would make it easier to add more store-related props in the future while keeping the interface clean.
Also applies to: 62-62, 179-184
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(list)/page.tsx(2 hunks)web/ce/hooks/store/index.ts(1 hunks)web/ce/hooks/store/use-page-store.ts(1 hunks)web/ce/hooks/store/use-page.ts(1 hunks)web/core/components/pages/dropdowns/actions.tsx(3 hunks)web/core/components/pages/list/block-item-action.tsx(2 hunks)web/core/components/pages/list/block.tsx(2 hunks)web/core/components/pages/list/root.tsx(1 hunks)web/core/components/pages/modals/delete-page-modal.tsx(1 hunks)web/core/hooks/store/index.ts(0 hunks)web/core/hooks/store/pages/index.ts(0 hunks)web/core/hooks/store/pages/use-page.ts(0 hunks)web/core/store/pages/base-page.ts(0 hunks)web/ee/hooks/store/index.ts(1 hunks)
💤 Files with no reviewable changes (4)
- web/core/hooks/store/index.ts
- web/core/hooks/store/pages/use-page.ts
- web/core/hooks/store/pages/index.ts
- web/core/store/pages/base-page.ts
✅ Files skipped from review due to trivial changes (2)
- web/ce/hooks/store/index.ts
- web/ee/hooks/store/index.ts
🔇 Additional comments (4)
web/core/components/pages/list/root.tsx (1)
7-28: LGTM! Clean refactor to use the new store hooks.The changes correctly implement the new store management approach:
- Added proper type for storeType
- Updated hook usage
- Correctly propagated storeType to child components
web/core/components/pages/list/block.tsx (1)
Line range hint
14-48: LGTM! Clean implementation of the new page hook.The changes correctly implement the new page hook:
- Proper type definitions
- Correct hook usage with object params
- Proper propagation of storeType
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(list)/page.tsx (1)
15-16: LGTM! Clean integration of the page store refactoring.The changes correctly integrate the new page store management by:
- Adding the required import in a dedicated section
- Setting the appropriate store type for the project pages context
Also applies to: 56-56
web/core/components/pages/list/block-item-action.tsx (1)
16-17: LGTM! Clean propagation of store type.The changes correctly propagate the store type through the component hierarchy while maintaining existing functionality.
Also applies to: 24-24, 28-28, 87-87
web/ce/hooks/store/use-page-store.ts
Outdated
| export const usePageStore = <T extends EPageStoreType>(storeType: T): TReturnType[T] => { | ||
| const context = useContext(StoreContext); | ||
| if (context === undefined) throw new Error("useProjectPage must be used within StoreProvider"); | ||
|
|
||
| if (storeType === EPageStoreType.PROJECT) { | ||
| return context.projectPages; | ||
| } | ||
|
|
There was a problem hiding this comment.
Fix incorrect default return and add exhaustive type checking.
The function has two issues:
- The default return always returns
projectPagesregardless ofstoreType - Missing exhaustive type checking for
storeType
Apply this diff to fix the issues:
export const usePageStore = <T extends EPageStoreType>(storeType: T): TReturnType[T] => {
const context = useContext(StoreContext);
if (context === undefined) throw new Error("useProjectPage must be used within StoreProvider");
if (storeType === EPageStoreType.PROJECT) {
return context.projectPages;
}
- return context.projectPages;
+ const _exhaustiveCheck: never = storeType;
+ throw new Error(`Unsupported store type: ${storeType}`);
};📝 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.
| export const usePageStore = <T extends EPageStoreType>(storeType: T): TReturnType[T] => { | |
| const context = useContext(StoreContext); | |
| if (context === undefined) throw new Error("useProjectPage must be used within StoreProvider"); | |
| if (storeType === EPageStoreType.PROJECT) { | |
| return context.projectPages; | |
| } | |
| export const usePageStore = <T extends EPageStoreType>(storeType: T): TReturnType[T] => { | |
| const context = useContext(StoreContext); | |
| if (context === undefined) throw new Error("useProjectPage must be used within StoreProvider"); | |
| if (storeType === EPageStoreType.PROJECT) { | |
| return context.projectPages; | |
| } | |
| const _exhaustiveCheck: never = storeType; | |
| throw new Error(`Unsupported store type: ${storeType}`); | |
| }; |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
web/core/components/pages/pages-list-main-content.tsx (1)
26-26: Consider adding prop validation for storeType.While the migration to
usePageStorelooks good, consider adding prop validation to ensurestoreTypeis always provided, as it's a required prop that the component depends on.export const PagesListMainContent: React.FC<Props> = observer((props) => { const { children, pageType, storeType } = props; + + if (!storeType) { + throw new Error('storeType is required for PagesListMainContent'); + } + // store hooks const { loader, isAnyPageAvailable, getCurrentProjectFilteredPageIds, getCurrentProjectPageIds, filters } = usePageStore(storeType);Also applies to: 29-29
web/core/components/pages/editor/page-root.tsx (1)
41-41: Add JSDoc documentation for the new prop.Consider adding documentation to explain the purpose and possible values of
storeType. This will help other developers understand how to use this prop correctly.+ /** Type of page store this component should use (e.g., project, workspace) */ storeType: EPageStoreType;web/core/components/pages/editor/header/mobile-root.tsx (1)
8-9: LGTM! Consider using Pick/Omit for prop types.The changes consistently implement the new
storeTypeprop throughout the component hierarchy. The implementation is type-safe and follows a clear pattern.Consider using TypeScript's utility types to define props that are passed down:
type Props = { editorRef: EditorRefApi; page: TPageInstance; setSidePeekVisible: (sidePeekState: boolean) => void; sidePeekVisible: boolean; } & Pick<PageExtraOptionsProps, 'storeType'>;Also applies to: 18-18, 22-22, 39-39
web/core/components/pages/editor/header/root.tsx (1)
10-11: LGTM! Consider adding JSDoc for the component.The changes consistently implement and propagate the
storeTypeprop. The implementation is type-safe and follows React best practices.Consider adding JSDoc documentation to clarify the component's purpose and prop usage:
/** * Root component for the page editor header. * @param props.storeType - Type of page store to interact with * @param props.editorRef - Reference to the editor instance * ... */ export const PageEditorHeaderRoot: React.FC<Props> = observer((props) => {Also applies to: 21-21, 25-25, 56-56, 64-64
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(list)/header.tsx (1)
Line range hint
34-52: Enhance error handling in page creation.The current error handling could be more robust by:
- Handling specific error types
- Providing more detailed error messages
- Logging errors for debugging
Consider this improvement:
await createPage(payload) .then((res) => { + if (!res?.id) throw new Error("Invalid response: missing page ID"); const pageId = `/${workspaceSlug}/projects/${currentProjectDetails?.id}/pages/${res?.id}`; router.push(pageId); }) .catch((err) => { + console.error("Failed to create page:", err); + const errorMessage = err?.data?.error || + (err.message === "Invalid response: missing page ID" + ? "Invalid server response while creating page" + : "Page could not be created. Please try again."); setToast({ type: TOAST_TYPE.ERROR, title: "Error!", - message: err?.data?.error || "Page could not be created. Please try again.", + message: errorMessage, }) })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx(3 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/header.tsx(2 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(list)/header.tsx(2 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(list)/page.tsx(2 hunks)web/ce/components/command-palette/modals/project-level.tsx(2 hunks)web/ce/hooks/store/use-page-store.ts(1 hunks)web/core/components/pages/editor/header/extra-options.tsx(2 hunks)web/core/components/pages/editor/header/mobile-root.tsx(3 hunks)web/core/components/pages/editor/header/options-dropdown.tsx(2 hunks)web/core/components/pages/editor/header/root.tsx(3 hunks)web/core/components/pages/editor/page-root.tsx(3 hunks)web/core/components/pages/header/root.tsx(1 hunks)web/core/components/pages/modals/create-page-modal.tsx(3 hunks)web/core/components/pages/pages-list-main-content.tsx(1 hunks)web/core/components/pages/pages-list-view.tsx(2 hunks)web/core/hooks/use-favorite-item-details.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- web/ce/hooks/store/use-page-store.ts
- web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(list)/page.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (29)
web/core/components/pages/header/root.tsx (2)
18-20: LGTM! Clean refactoring of imports and typesThe changes align well with the PR's objective of introducing generic hooks. The new imports and type additions provide a more flexible approach to page store management.
Also applies to: 25-25
30-30: Verify consistent store type usage across componentsThe store implementation looks good, but let's verify that all components using this header component are passing the correct store type.
Also applies to: 32-32
✅ Verification successful
Store type usage is consistent and complete
The verification shows that the store type is correctly passed in all component usages, and no legacy
useProjectPageshooks remain in the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all instances where PagesListHeaderRoot is used # to ensure storeType prop is correctly passed # Find usage of PagesListHeaderRoot rg -l "PagesListHeaderRoot" | while read -r file; do echo "=== $file ===" # Show the component usage with context rg -A 5 -B 5 "PagesListHeaderRoot" "$file" done # Also check for any remaining useProjectPages usage that might need updating rg "useProjectPages"Length of output: 1530
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/header.tsx (2)
18-18: LGTM! Import changes align with the generic hooks approach.The replacement of specific page hooks with generic ones (
EPageStoreType,usePage) aligns well with the PR's refactoring objectives.Also applies to: 23-24
37-40: Verify consistent hook usage across components.The implementation correctly uses the new generic
usePagehook with appropriate parameters. Let's verify that this pattern is consistently followed in other components.✅ Verification successful
Hook migration successfully verified
All instances of
usePagehook are consistently implemented with properstoreTypeandpageIdparameters. The olduseProjectPagehook has been fully migrated.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining uses of the old useProjectPage hook # and verify consistent usage of the new usePage hook with storeType # Check for any remaining useProjectPage imports echo "Checking for remaining useProjectPage imports..." rg "useProjectPage" # Verify consistent usage of usePage with storeType echo "Verifying usePage hook usage pattern..." ast-grep --pattern 'usePage({ $$$, storeType: EPageStoreType.$_, $$$ })'Length of output: 1747
web/core/components/pages/pages-list-main-content.tsx (3)
12-14: LGTM! Clean import organization.The new imports are well-organized and properly aligned with the refactoring objectives.
22-22: LGTM! Good type safety with enum.The addition of
storeTypeusingEPageStoreTypeenum provides good type safety and makes the component more flexible.
24-30: Verify all usages of PagesListMainContent include storeType prop.Let's ensure all instances of this component usage have been updated with the new required prop.
✅ Verification successful
All usages of PagesListMainContent include required storeType prop
The component is used correctly with all required props in its single instance within
pages-list-view.tsx.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all usages of PagesListMainContent to verify storeType prop ast-grep --pattern 'PagesListMainContent $$$' -l ts,tsx # Search for potential instances where storeType might be missing rg -U "PagesListMainContent[^}]*?>" -A 2 -g '*.{ts,tsx}'Length of output: 721
Script:
#!/bin/bash # Search for PagesListMainContent usage with proper context rg -U "<PagesListMainContent.*?(?s).*?</PagesListMainContent>" -A 2 -B 2 --multiline -g '*.{ts,tsx}' # Backup search to catch any other usage patterns rg -U "PagesListMainContent" -A 5 -B 5 -g '*.{ts,tsx}'Length of output: 3250
web/core/hooks/use-favorite-item-details.tsx (3)
11-13: LGTM! Clean import organization.The import statements are well-organized and clearly separate core hooks from plane-web hooks, maintaining consistency with the codebase style.
28-31: LGTM! Clean hook implementation.The refactoring to use
usePagewith named parameters improves code clarity and aligns with the generic hooks approach.
28-31: Verify the hardcoded store type.Please confirm that
EPageStoreType.PROJECTis the correct store type for all favorite page items. Consider if there are cases where different page types might need different store types.Let's verify the usage:
✅ Verification successful
The hardcoded store type is correct and intentional.
The
EPageStoreType.PROJECTis the only available store type in the enum and is used consistently for all page entities, including favorite page items.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other usages of usePage to verify consistent store type usage rg "usePage.*storeType:" -A 2 # Search for all enum values to understand available options ast-grep --pattern 'enum EPageStoreType { $$$ }'Length of output: 601
Script:
#!/bin/bash # Search for favorite item related code rg "FavoriteItem" -A 2 # Search for any type definitions related to favorite items ast-grep --pattern 'type $_ = { $$$ }'Length of output: 65724
web/core/components/pages/pages-list-view.tsx (3)
6-7: LGTM! Import changes align with the new store architecture.The addition of
EPageStoreTypeandusePageStoreimports supports the transition to generic page store hooks.
32-41: LGTM! Child component prop updates are consistent.The
storeTypeprop is correctly passed down to bothPagesListHeaderRootandPagesListMainContentcomponents, maintaining consistency with the new store architecture.
9-14: Verify type consistency across components.The addition of
storeType: EPageStoreTypetoTPageViewlooks good. Let's verify that all components consuming this type have been updated accordingly.✅ Verification successful
Type consistency verified - no issues found
The
TPageViewtype is locally scoped to thepages-list-view.tsxcomponent and is not imported or used elsewhere in the codebase. The component implementation correctly handles all type properties, including the newstoreType. No further updates are needed.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for components that might be using TPageView without storeType rg "TPageView" --type typescriptLength of output: 67
Script:
#!/bin/bash # Search for TPageView in TypeScript and TSX files with context rg "TPageView" -t ts -t tsx -C 2 # Search for files importing or using this type ast-grep --pattern 'import { $$$TPageView$$$ } from "$_"'Length of output: 120
Script:
#!/bin/bash # Search for TPageView in .ts and .tsx files using file extension pattern rg "TPageView" --glob "*.{ts,tsx}" -C 2 # Search for type definition pattern ast-grep --pattern 'type TPageView = { $$$ }'Length of output: 1377
web/core/components/pages/editor/page-root.tsx (3)
21-22: LGTM! Clean import addition.The import of
EPageStoreTypealigns with the PR's objective of introducing generic page store hooks.
46-46: LGTM! Clean props destructuring.The
storeTypeprop is correctly destructured and follows the established ordering pattern.
112-112: Verify PageEditorHeaderRoot's prop type definition.Ensure that the
PageEditorHeaderRootcomponent's props interface includes thestoreTypeprop with the correct type.✅ Verification successful
Props interface already includes storeType
The
PageEditorHeaderRootcomponent already has the correct prop type definition forstoreType: EPageStoreTypein its Props interface, and the prop is properly handled in the component.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for PageEditorHeaderRoot's type definition ast-grep --pattern 'type $name = { $$$ storeType: EPageStoreType; $$$ }'Length of output: 80
Script:
#!/bin/bash # Find the component file fd "page-editor-header-root" --type f # Once found, examine its content fd "page-editor-header-root" --type f --exec cat {}Length of output: 91
Script:
#!/bin/bash # Search for the component definition rg "PageEditorHeaderRoot" -A 5 -B 5 # Also search for any type/interface definitions related to it rg "interface.*PageEditorHeaderRoot" -A 5 rg "type.*PageEditorHeaderRoot" -A 5Length of output: 2773
web/ce/components/command-palette/modals/project-level.tsx (1)
9-10: LGTM! Appropriate use of hardcoded store type.The changes correctly implement the store type for project-level modals. Using
EPageStoreType.PROJECTas a hardcoded value is appropriate here since this component specifically handles project-level operations.Also applies to: 58-58
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(list)/header.tsx (1)
16-16: LGTM! Clean transition to generic hook.The change from
useProjectPagestousePageStorewithEPageStoreType.PROJECTmaintains the same functionality while aligning with the new generic approach.Also applies to: 19-20, 32-32
web/core/components/pages/modals/create-page-modal.tsx (3)
13-16: LGTM! Clean import organization.The imports are well-organized, with clear separation between hooks and plane-web specific imports.
Line range hint
18-26: Props interface update looks good.The addition of
storeTypeprop and its proper typing withEPageStoreTypealigns well with the generic store approach. The props destructuring is clean and maintains good code organization.Also applies to: 29-37
47-47: Verify hook migration completeness.The migration from
useProjectPagestousePageStorelooks correct, but let's verify that all usages of the old hook have been updated consistently across the codebase.✅ Verification successful
Migration verification completed successfully
The migration from
useProjectPagestousePageStorehas been fully completed across the codebase. All instances are consistently using the new pattern withEPageStoreType.PROJECT.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining instances of useProjectPages rg "useProjectPages" # Search for usePageStore usage patterns rg "usePageStore\((EPageStoreType\.[A-Z_]+)\)"Length of output: 551
Script:
#!/bin/bash # Search for partial matches of old hook name rg -i "projectpages?" # Search for all usePageStore variations rg "usePageStore" # Search for the modal file to see storeType definition rg -A 5 "storeType.*=" "web/core/components/pages/modals/create-page-modal.tsx"Length of output: 11900
web/core/components/pages/editor/header/options-dropdown.tsx (2)
18-19: Props update is consistent with the refactoring pattern.The addition of
storeTypeto Props maintains consistency with the new store management approach.Also applies to: 26-26
120-120: Verify PageActions component compatibility.The
storeTypeprop is correctly passed to PageActions. Let's verify that PageActions component is updated to handle this prop.✅ Verification successful
PageActions component is compatible with storeType prop
The PageActions component in
web/core/components/pages/dropdowns/actions.tsxis properly typed to handle thestoreTypeprop of typeEPageStoreType.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check PageActions component implementation ast-grep --pattern 'type Props = { $$$ storeType: EPageStoreType; $$$ }'Length of output: 4704
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx (2)
161-161: Consistent store type propagation.The
storeTypeprop is correctly passed to PageRoot, maintaining consistency with the new store management approach.
40-44: Hook migration looks good but verify error handling.The migration from project-specific hooks to generic hooks is clean. The
usePageStoreandusePagehooks are correctly configured withEPageStoreType.PROJECT.✅ Verification successful
Error handling is properly maintained in the hooks migration
The store-level error handling with provider validation and type-safe store selection is sufficient. The consistent usage of
EPageStoreType.PROJECTacross components ensures type safety.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check error handling patterns in the new hooks ast-grep --pattern 'try { $$$ usePageStore($$$) $$$ } catch' # Verify consistent store type usage rg "EPageStoreType\.PROJECT" -A 2 -B 2Length of output: 6137
web/core/components/pages/editor/header/extra-options.tsx (4)
15-16: LGTM! Clean import organization.The import follows the established pattern of categorizing imports with comments.
23-23: LGTM! Well-typed prop addition.The
storeTypeprop is properly typed as a required enum, which is appropriate for core store functionality.
27-27: LGTM! Clean props destructuring.The destructuring follows the Props type definition and maintains a logical order.
90-90: Verify PageOptionsDropdown's handling of storeType.The
storeTypeprop is correctly passed to PageOptionsDropdown. However, we should verify that the receiving component properly handles different store types.Let's verify the PageOptionsDropdown implementation:
✅ Verification successful
storeType prop is properly implemented in PageOptionsDropdown
The component correctly handles the storeType prop through proper typing, destructuring, and forwarding to child components. The implementation follows React best practices and maintains type safety.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for PageOptionsDropdown component definition and its usage of storeType ast-grep --pattern 'export const PageOptionsDropdown = $_({ $$$, storeType }: $_) => { $$$ }'Length of output: 97
Script:
#!/bin/bash # Try different patterns to find PageOptionsDropdown ast-grep --pattern 'PageOptionsDropdown = $_' # Also search for type definitions ast-grep --pattern 'interface PageOptionsDropdownProps { $$$ }' # Backup search using ripgrep rg "PageOptionsDropdown" -A 10Length of output: 2568
Script:
#!/bin/bash # Find Props type definition ast-grep --pattern 'type Props = { $$$ }' # Look for storeType usage in the component rg "storeType" -A 3 "web/core/components/pages/editor/header/options-dropdown.tsx" # Get the full component implementation cat "web/core/components/pages/editor/header/options-dropdown.tsx"Length of output: 68390
Script:
#!/bin/bash # Search for storeType usage in options-dropdown.tsx rg "storeType" -B 2 -A 5 "web/core/components/pages/editor/header/options-dropdown.tsx" # Get imports to understand available store types rg "EPageStoreType" -A 3 "web/core/components/pages/editor/header/options-dropdown.tsx"Length of output: 849
| const { children, pageType, projectId, storeType, workspaceSlug } = props; | ||
| // store hooks | ||
| const { isAnyPageAvailable, getAllPages } = useProjectPages(); | ||
| const { isAnyPageAvailable, getAllPages } = usePageStore(EPageStoreType.PROJECT); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using the provided storeType prop.
While the component accepts a storeType prop, it's using a hardcoded EPageStoreType.PROJECT in usePageStore. Consider using the provided storeType prop for consistency:
-const { isAnyPageAvailable, getAllPages } = usePageStore(EPageStoreType.PROJECT);
+const { isAnyPageAvailable, getAllPages } = usePageStore(storeType);📝 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.
| const { children, pageType, projectId, storeType, workspaceSlug } = props; | |
| // store hooks | |
| const { isAnyPageAvailable, getAllPages } = useProjectPages(); | |
| const { isAnyPageAvailable, getAllPages } = usePageStore(EPageStoreType.PROJECT); | |
| const { children, pageType, projectId, storeType, workspaceSlug } = props; | |
| // store hooks | |
| const { isAnyPageAvailable, getAllPages } = usePageStore(storeType); |
Description
This PR creates a generic
usePageandusePageStorehooks to access actions and observables.Type of Change
Summary by CodeRabbit
New Features
Refactor
Chores