BA: improve relay mocks #156
Conversation
|
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/components/.storybook/decorators/withProviders.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/components/.eslintrc.js
WalkthroughThe pull request introduces several changes primarily focused on the Changes
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: 0
🧹 Outside diff range and nitpick comments (4)
packages/graphql/utils/createTestEnvironment/types.ts (1)
6-6: LGTM! Consider adding JSDoc comments.The type changes look good and maintain consistency. The addition of
queryNameto both types and the newQueueOperationResolverParamsexport improves the flexibility of the test environment configuration.Consider adding JSDoc comments to document the purpose of
queryNameand when it should be used. Example:type MockResolvers = { mockResolvers: MockPayloadGenerator.MockResolvers data?: never + /** Optional query name to identify specific operations in the resolver */ queryName?: string }Also applies to: 12-12, 16-17
packages/components/.storybook/decorators/withProviders.tsx (1)
19-19: TODO comment needs attention.The comment about Tailwind classes registration should be addressed or tracked properly.
Would you like me to help create an issue to track the Tailwind classes registration problem?
packages/graphql/utils/createTestEnvironment/index.ts (2)
41-46: Consider removing or conditionally enabling debug logsThe multiple console.log statements add noise to test output. Consider:
- Removing them if they were for development
- Or adding a debug flag parameter to control logging
- console.log('queueOperationResolver', mockResolvers, data, queryName) - environment.mock.queueOperationResolver((operationToResolve) => { - console.log('AOPS', operationToResolve) - if (mockResolvers) { - console.log('mockResolvers', mockResolvers) + environment.mock.queueOperationResolver((operationToResolve) => { + if (mockResolvers) {
35-59: Consider restructuring the resolver logic for better maintainabilityThe current implementation has nested conditionals that could be simplified. Consider restructuring to reduce complexity and improve readability.
const queueOperationResolver = ({ mockResolvers, data, queryName, }: QueueOperationResolverParams) => { try { environment.mock.queueOperationResolver((operationToResolve) => { - if (mockResolvers) { - return MockPayloadGenerator.generate(operationToResolve, mockResolvers) - } - - // return the data mock if no queryName is provided - if (data && !queryName) return data - - // return the data mock if the queryName is provided and the operation matches - if (queryName && operationToResolve.fragment.node.name === queryName) { - return data - } - - console.warn('The operation was not mocked.') - return null + if (mockResolvers) { + return MockPayloadGenerator.generate(operationToResolve, mockResolvers) + } + + const operationName = operationToResolve.fragment.node.name + const shouldReturnData = !queryName || operationName === queryName + + if (data && shouldReturnData) { + return data + } + + console.warn('The operation was not mocked.') + return null }) } catch (e) { console.error(e) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/components/.storybook/decorators/withProviders.tsx(1 hunks)packages/components/CHANGELOG.md(1 hunks)packages/components/modules/navigations/Header/AccountMenu/AccountPopover/__storybook__/stories.tsx(1 hunks)packages/components/package.json(1 hunks)packages/graphql/CHANGELOG.md(1 hunks)packages/graphql/package.json(1 hunks)packages/graphql/utils/createTestEnvironment/index.ts(2 hunks)packages/graphql/utils/createTestEnvironment/types.ts(1 hunks)packages/wagtail/CHANGELOG.md(1 hunks)packages/wagtail/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- packages/graphql/package.json
- packages/wagtail/package.json
- packages/components/package.json
- packages/wagtail/CHANGELOG.md
🔇 Additional comments (4)
packages/components/modules/navigations/Header/AccountMenu/AccountPopover/__storybook__/stories.tsx (1)
37-37: LGTM! Verify mock resolver integration.
The update to use object parameter syntax for queueOperationResolver is consistent with the type changes.
Let's verify that all stories are using the new parameter format:
✅ Verification successful
Mock resolver integration verified successfully
The search revealed that all story files are using the new object parameter syntax for queueOperationResolver. The only instance found with different syntax is in createTestEnvironment/index.ts, which is actually the implementation of the resolver itself, where it's correctly using a callback function as required by the environment mock API.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining direct calls to queueOperationResolver
rg "queueOperationResolver\([^{]" --type ts --type tsx
Length of output: 84
Script:
#!/bin/bash
# Search for any remaining direct calls to queueOperationResolver with proper file type detection
rg "queueOperationResolver\([^{]" -t typescript -t tsx
Length of output: 91
Script:
#!/bin/bash
# Search for any direct calls to queueOperationResolver without object parameter syntax
# Using just the file extensions since ripgrep type detection seems unavailable
rg "queueOperationResolver\([^{]" --glob "*.ts" --glob "*.tsx"
Length of output: 190
Script:
#!/bin/bash
# Let's check the context of this usage to understand if it's a different use case
rg "queueOperationResolver" -B 2 -A 2 packages/graphql/utils/createTestEnvironment/index.ts
Length of output: 539
packages/components/.storybook/decorators/withProviders.tsx (1)
24-28: LGTM! Clean implementation of resolver configuration.
The changes effectively simplify the control flow by:
- Properly handling undefined values with fallbacks
- Using destructuring for clean parameter access
- Consolidating the resolver logic into a single call
Let's verify the parameters are being passed correctly from stories:
✅ Verification successful
The queryName parameter implementation is correctly used in the test environment
The search results show that queryName is properly implemented in the test environment (packages/graphql/utils/createTestEnvironment/index.ts) where it's used to:
- Match specific operations in the resolver
- Handle undefined cases with appropriate fallbacks
- Control mock data resolution based on operation names
The parameter is also correctly typed in the test environment types file, and there are no story files using these parameters directly, which is expected as this is primarily a test utility. The other occurrences of queryName in the codebase are related to GraphQL @refetchable directives, which is a different usage altogether.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for story parameters that might need updating
rg "parameters:\s*{[^}]*queryName" --type ts --type tsx
Length of output: 184
Script:
#!/bin/bash
# Search for story parameters using queryName with proper regex
rg "parameters:\s*\{[^}]*queryName" --type ts --type tsx
# Also search for direct usage of queryName in story files
rg "queryName:" --type ts --type tsx
# Look for story files that might use these parameters
fd "\.stories\.(ts|tsx)$" --exec grep -l "parameters"
Length of output: 205
Script:
#!/bin/bash
# Search for story files and their content
fd -e ts -e tsx ".stories." -x cat {} \; | grep -A 5 -B 5 "queryName"
# Also search for any file containing queryName parameter usage
rg "queryName" -A 5 -B 5
Length of output: 14143
packages/graphql/CHANGELOG.md (1)
3-9: LGTM! Changelog entry is accurate and well-formatted
The changelog entry correctly describes the adaptation of queueOperationResolver to handle both resolvers and data mocks.
packages/components/CHANGELOG.md (1)
3-10: LGTM! Changelog entries are accurate and well-structured
The changelog correctly documents both the adaptation of Storybook's withProviders and the dependency update.
graphqlpackage update -v 1.1.13queueOperationResolverto deal with resolvers or data mocks.componentspackage update -v 0.0.30queueOperationResolver.wagtailpackage update -v 1.0.6Summary by CodeRabbit
New Features
withProvidersfunctionality in Storybook for improved operation handling.queueOperationResolverto manage both resolvers and data mocks effectively.Bug Fixes
RoomListSubscriptionconnection and improved accessibility and alignment in various components.Documentation
@baseapp-frontend/componentsand@baseapp-frontend/graphqlpackages to reflect recent changes and version updates.Chores
@baseapp-frontend/components,@baseapp-frontend/graphql, and@baseapp-frontend/wagtailpackages.