🤖 refactor: split workspaces into WorkspaceContext with tests #605
+1,520
−519
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kylecarbs
force-pushed
the
apply-pr-600-to-workspaces
branch
2 times, most recently
from
3b95f25 to
3fcb45f
Compare
Following the pattern from PR #600 (projects split), this moves workspace state management from useWorkspaceManagement hook into a dedicated WorkspaceContext. - Created WorkspaceContext with workspace metadata, operations (create/remove/rename), and selection state - Comprehensive test coverage (14 tests) matching ProjectContext test patterns - Updated AppLoader to use WorkspaceProvider wrapper - Maintains same API surface for existing components via AppContext pass-through - Tests verify metadata loading, CRUD operations, event subscriptions, and error handling This eliminates prop drilling and centralizes workspace state management, making it easier to test and maintain workspace operations independently.
- Add eslint disable comments for test file mocks (consistent with PR #600 pattern) - Fix react-hooks/exhaustive-deps warnings by destructuring props in effects - Change empty arrow function to use comment (no-op setWorkspaceMetadata) - Remove await from non-promise expect call in tests
App.tsx needs to optimistically update workspace metadata when creating/forking workspaces to avoid race condition with validation effect. WorkspaceContext now: - Exposes setWorkspaceMetadata to allow App.tsx to update metadata immediately - Updates metadata map immediately in createWorkspace before returning - Passes through setWorkspaceMetadata in AppLoader instead of no-op This ensures metadata is available when setSelectedWorkspace is called, preventing the validation effect from clearing the selection before the async subscription fires.
ProjectSidebar, LeftSidebar, and App.tsx now use WorkspaceContext directly instead of passing workspace state/operations through props: - ProjectSidebar uses useWorkspaceContext() to get workspace operations - LeftSidebar no longer passes workspace props to ProjectSidebar - App.tsx no longer passes workspace callbacks to LeftSidebar - Telemetry tracking moved to effect watching selectedWorkspace changes This eliminates prop drilling for: - selectedWorkspace / setSelectedWorkspace - onAddWorkspace (beginWorkspaceCreation) - onRemoveWorkspace / onRenameWorkspace - workspaceMetadata Cleaned up unused callbacks and imports.
- Remove useProjectManagement in favor of useProjectContext - AppLoader now wraps with both ProjectProvider and WorkspaceProvider - App.tsx and LeftSidebar.tsx get projects from ProjectContext - Fix lint/typecheck errors from integration
ProjectSidebar, LeftSidebar, and App.tsx now get project operations from ProjectContext instead of passing through props: - ProjectSidebar uses useProjectContext() for project CRUD and secrets - LeftSidebar no longer passes project props to ProjectSidebar - App.tsx no longer passes project callbacks to LeftSidebar Eliminated prop drilling for: - projects (Map) - onAddProject / onRemoveProject - onGetSecrets / onUpdateSecrets This completes the prop drilling elimination - workspace operations come from WorkspaceContext, project operations come from ProjectContext.
This hook is now obsolete - all workspace management is handled by WorkspaceContext instead.
WorkspaceContext now owns all workspace state including selectedWorkspace: - Uses usePersistedState internally for localStorage persistence - Handles URL hash restoration (#workspace=<id>) - Handles launch project detection (--add-project flag) Benefits: - Single source of truth for all workspace state - Simplified AppLoader (eliminated AppLoaderMiddle layer) - Cleaner separation of concerns - ~60 net lines removed Tests updated to set selectedWorkspace via context API instead of props.
kylecarbs
force-pushed
the
apply-pr-600-to-workspaces
branch
from
69bf899 to
1cf729e
Compare
AppLoader was calling useProjectContext() before ProjectProvider was rendered, breaking Storybook stories. Restored the middle component to call hooks inside the provider.
Eliminated onProjectsUpdate prop by having WorkspaceProvider call useProjectContext() directly. This removes the need for AppLoaderMiddle and simplifies the component tree. Benefits: - WorkspaceProvider is self-contained - No callback prop drilling - Cleaner AppLoader structure - ~30 net lines removed
AppContext was just a pass-through wrapper for WorkspaceContext data. Components now call useWorkspaceContext() directly instead of useApp(). Changes: - Deleted AppContext.tsx (~57 lines) - Updated App.tsx and useSortedWorkspacesByProject to use useWorkspaceContext - Removed AppProvider wrapper from AppLoader - Simplified component tree Benefits: - ~70 net lines removed - One less layer of indirection - Direct context access instead of prop pass-through - Clearer ownership - WorkspaceContext owns all workspace state
- Consolidated createMockAPI and undefined createMockProjectsAPI into single function - Updated all 21 tests to use unified mock structure - Fixed all TypeScript and eslint violations properly - Removed all eslint-disable comments from file header - Added proper types for mock APIs - All tests passing (21/21), zero eslint errors _Generated with `mux`_
_Generated with `mux`_
happy-dom's Document type has minor differences from standard DOM Document. Adding explicit cast to satisfy TypeScript while maintaining test functionality.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Following the pattern from PR #600, this moves workspace state management from
useWorkspaceManagementhook into a dedicatedWorkspaceContext.Changes
Created WorkspaceContext:
Comprehensive test coverage:
Updated AppLoader:
Testing
bun test src/contexts/WorkspaceContext.test.tsx make typecheckAll tests pass, typechecks clean.
Generated with
mux