Ba 2287 fe unify test and storybook providers #244
Conversation
priscilladeroode
requested review from
anicioalexandre and
deboracosilveira
as code owners
|
WalkthroughThis PR unifies and refines provider setups across test and Storybook configurations and standardizes dependency versioning. Multiple package.json files and changelogs have been updated, while numerous module exports have been converted from CommonJS to ES Module syntax. New test utility and provider components have been added and old, redundant ones removed. In addition, import paths and configuration files (tsconfig, tsup) have been adjusted to include the tests directory. Overall, the changes streamline the testing, storybook providers, and build process. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Component
participant HOC as Unified Provider HOC
participant Auth as AuthProvider
participant GraphQL as GraphQLProvider
participant DS as DesignSystemProvider
C->>HOC: Wrapped via withComponentCompleteTestProviders
HOC->>Auth: Inject authentication context
HOC->>GraphQL: Inject GraphQL context
HOC->>DS: Inject design system context
DS-->>C: Render with all providers in place
Possibly related PRs
Suggested labels
Poem
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/authentication/__mocks__/expo-constants.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/authentication/.eslintrc.js
packages/authentication/__mocks__/expo-modules-core.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/authentication/.eslintrc.js
packages/authentication/__mocks__/expo-secure-store.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/authentication/.eslintrc.js
✨ Finishing Touches
🪧 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 comments (2)
packages/design-system/tests/commom/index.ts (1)
1-2:⚠️ Potential issueFile is incomplete - missing exports
The file contains a comment indicating it should export common design-system tests, but there are no actual exports implemented. Based on the PR objectives to unify test and Storybook providers, this file should likely export shared provider components or utility functions.
Additionally, note that the directory name has a typo: "commom" instead of "common".
Consider implementing the necessary exports for the common test utilities:
// exports common design-system tests export { withDesignSystemTestProviders } from './providers';packages/design-system/tests/native/index.ts (1)
1-2:⚠️ Potential issueFile is empty but claims to export providers
The file contains a comment indicating it should export native design-system providers, but there are no actual exports implemented. This appears to be either an incomplete implementation or a placeholder.
Either implement the necessary exports or add a TODO comment with a ticket reference explaining why this is empty. If this is intentional, consider adding more context in the comment.
🧹 Nitpick comments (10)
packages/utils/constants/compose.ts (1)
1-6: Improve type safety and add documentation to the compose utilityWhile the implementation is functional and directly supports the PR objective of creating a shared wrapper for providers, the use of
anytypes reduces type safety.Consider refactoring with stronger typing and documentation:
- import { FC } from 'react' + import { ComponentType, FC } from 'react' - export const compose = - (...hocs: any[]) => - (Component: FC<any>) => - hocs.reduceRight((AccumulatedComponent, hoc) => hoc(AccumulatedComponent), Component) + /** + * Composes multiple higher-order components into a single higher-order component. + * The composition applies HOCs from right to left (last to first in the argument list). + * + * @param hocs - Array of higher-order components to compose + * @returns A function that takes a component and returns the composed component + * + * @example + * const EnhancedComponent = compose( + * withTheme, + * withRouter, + * withRedux + * )(BaseComponent); + */ + export const compose = + <P extends object>(...hocs: Array<(Component: ComponentType<any>) => ComponentType<any>>) => + (Component: ComponentType<P>): ComponentType<P> => + hocs.reduceRight((AccumulatedComponent, hoc) => hoc(AccumulatedComponent), Component)packages/authentication/package.json (1)
22-23: Consider moving Storybook to devDependenciesWhile adding Storybook as a dependency aligns with the PR objectives, it appears this package doesn't actively use Storybook (as indicated by the "No storybook found" message in the storybook script on line 13).
Since Storybook is a development tool rather than a runtime dependency, consider moving it to the devDependencies section:
"dependencies": { "@hookform/resolvers": "catalog:", "@tanstack/react-query": "catalog:", "jotai": "catalog:", "js-cookie": "catalog:", "react-hook-form": "catalog:", - "zod": "catalog:", - "@storybook/react": "catalog:storybook" + "zod": "catalog:" }, "peerDependencies": { "@baseapp-frontend/utils": "workspace:*", "react": "catalog:react18" }, "devDependencies": { "@baseapp-frontend/config": "workspace:*", "@baseapp-frontend/eslint-plugin": "workspace:*", "@baseapp-frontend/test": "workspace:*", "@baseapp-frontend/tsconfig": "workspace:*", "@faker-js/faker": "catalog:test", "@testing-library/jest-dom": "catalog:test", "@testing-library/react": "catalog:test", "@testing-library/user-event": "catalog:test", + "@storybook/react": "catalog:storybook", "@types/jest": "catalog:test",packages/utils/functions/expo/index.ts (1)
19-23: Remove commented-out code.The commented-out code is now redundant since it's been replaced by the try-catch implementation above.
- // const Constants = require('expo-constants').default - - // if (!Constants?.expoConfig?.extra) return undefined - - // return (Constants.expoConfig.extra as T)[key]packages/components/modules/tests/web/utils/withComponentCompleteTestProviders/index.tsx (1)
8-15: Excellent use of composition pattern for provider wrapping.The
composeutility effectively combines multiple provider HOCs into a single reusable wrapper. This approach is clean, maintainable, and directly addresses the PR's goal of creating a shared wrapper for all required providers.Consider adding JSDoc comments to document the purpose and usage of this function.
+/** + * Combines all test providers into a single HOC + * This wrapper provides context for components, authentication, GraphQL, and design system + * @returns A higher-order component that wraps a component with all necessary test providers + */ const withComponentCompleteTestProviders = compose( withComponentTestProviders, withAuthenticationTestProviders, withGraphqlTestProviders, withDesignSystemTestProviders, )packages/test/providers/withStorybookProvidersWrapper/index.tsx (1)
6-9: Consider adding JSDoc comments for improved documentationSince this function serves as an important bridging utility between test providers and Storybook, it would benefit from JSDoc comments explaining its purpose, parameters, and return value. This would help other developers understand how to use it correctly.
+/** + * Higher-order component that wraps a Storybook story with a provided test provider. + * @param provider - A function that wraps a React component with providers + * @returns A function that applies the provider to a Storybook story + */ const withTestProviders = (provider: (Component: FC<any>) => any) => (Story: StoryFn, context: StoryContext) => { const StoryWithProviders = provider(Story) return <StoryWithProviders context={context} /> }packages/authentication/modules/tests/utils/providers/withAuthenticationTestProviders/index.tsx (1)
21-21: Consider using type-safe prop forwardingThe type assertion
{...(props as Props)}could potentially be unsafe. Consider using a more type-safe approach for prop forwarding if possible, such as destructuring and explicitly passing the required props.packages/graphql/tests/utils/providers/withGraphqlTestProviders/index.tsx (2)
11-13: Consider adding a null check for context parametersThe code extracts values from
context?.parameterswithout checking ifparametersexists. While the optional chaining oncontext?helps avoid errors when context is null, it doesn't protect against cases where context exists butparametersis undefined.- const queryName = context?.parameters?.queryName || undefined - const mockResolvers = context?.parameters?.mockResolvers || undefined - const mockData = context?.parameters?.mockData || {} + const queryName = context?.parameters?.queryName || undefined + const mockResolvers = context?.parameters?.mockResolvers || undefined + const mockData = context?.parameters?.mockData || {}
23-25: Be cautious about directly mutating the context objectThe code directly mutates the
contextobject by settingcontext.parameters.environment. This could potentially cause issues if the object is frozen or intended to be immutable.Consider using a safer approach:
- if (context && !context.parameters.environment) { - context.parameters.environment = relayMockEnvironment - } + if (context && context.parameters && !context.parameters.environment) { + // Consider creating a new context object instead of mutating + context.parameters = { + ...context.parameters, + environment: relayMockEnvironment + } + }packages/design-system/tests/web/utils/providers/withDesignSystemTestProviders/index.tsx (2)
18-18: Consider memoizing the emotion cacheCreating a new Emotion cache on each render is inefficient. Consider using
useMemoor moving the cache creation outside the component to avoid recreating it on every render.- const emotionCache = createCache({ key: 'test-key' }) + // Move outside component or use useMemo + const emotionCache = createCache({ key: 'test-key' }) // Alternative with useMemo inside component: + import { useMemo } from 'react' + // ... + const emotionCache = useMemo(() => createCache({ key: 'test-key' }), [])
16-17: Avoid type assertions when possibleUsing type assertions (
as) can hide type errors. Consider restructuring the code to avoid the type assertion.- const { ThemeTestProviderProps, ReactSuspenseWithLoadingProps, ...restProps } = props as Props & - WithDesignSystemProviderProps + const { ThemeTestProviderProps, ReactSuspenseWithLoadingProps, ...restProps } = props
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/components/__generated__/ActivityLogsFragment.graphql.tsis excluded by!**/__generated__/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (84)
package.json(1 hunks)packages/authentication/__mocks__/expo-constants.ts(1 hunks)packages/authentication/index.ts(1 hunks)packages/authentication/modules/profile/index.ts(0 hunks)packages/authentication/modules/tests/utils/index.ts(1 hunks)packages/authentication/modules/tests/utils/providers/InitialProfileProviderForTesting/index.tsx(1 hunks)packages/authentication/modules/tests/utils/providers/InitialProfileProviderForTesting/types.ts(1 hunks)packages/authentication/modules/tests/utils/providers/withAuthenticationTestProviders/index.tsx(1 hunks)packages/authentication/modules/tests/utils/providers/withAuthenticationTestProviders/types.ts(1 hunks)packages/authentication/package.json(1 hunks)packages/authentication/services/auth.ts(1 hunks)packages/authentication/services/user.ts(1 hunks)packages/components/.storybook/decorators/index.ts(0 hunks)packages/components/.storybook/decorators/withProviders.tsx(0 hunks)packages/components/.storybook/preview.ts(2 hunks)packages/components/__mocks__/expo-constants.ts(1 hunks)packages/components/__mocks__/expo-modules-core.ts(1 hunks)packages/components/__mocks__/expo-secure-store.ts(1 hunks)packages/components/__mocks__/fetch.ts(1 hunks)packages/components/__mocks__/file.ts(1 hunks)packages/components/__mocks__/react-native.ts(1 hunks)packages/components/__mocks__/style.ts(1 hunks)packages/components/__test_utils__/withProviders/index.tsx(0 hunks)packages/components/__test_utils__/withProviders/types.ts(0 hunks)packages/components/baseapp-frontend-imports.d.ts(1 hunks)packages/components/modules/comments/web/Comments/__tests__/__utils__/CommentsForTesting/index.tsx(2 hunks)packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__storybook__/stories.tsx(0 hunks)packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/AccountPopover.cy.tsx(3 hunks)packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/AccountPopoverForTesting/index.tsx(1 hunks)packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/__mocks__/profiles.ts(1 hunks)packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/withProviders/index.tsx(0 hunks)packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/withProviders/types.ts(0 hunks)packages/components/modules/navigations/web/Header/__storybook__/stories.tsx(1 hunks)packages/components/modules/navigations/web/NavMini/__storybook__/stories.tsx(1 hunks)packages/components/modules/navigations/web/NavVertical/__storybook__/stories.tsx(1 hunks)packages/components/modules/navigations/web/NavigationLayout/__storybook__/stories.tsx(1 hunks)packages/components/modules/notifications/web/NotificationsList/__storybook__/stories.tsx(1 hunks)packages/components/modules/notifications/web/NotificationsPopover/__tests__/NotificationsPopoverForTesting/index.tsx(1 hunks)packages/components/modules/profiles/web/ProfileMembers/__tests__/__utils__/MembersForTesting/index.tsx(1 hunks)packages/components/modules/tests/web/index.ts(1 hunks)packages/components/modules/tests/web/utils/withComponentCompleteTestProviders/index.tsx(1 hunks)packages/components/modules/tests/web/utils/withComponentTestProviders/index.tsx(1 hunks)packages/components/modules/tests/web/utils/withComponentTestProviders/types.ts(1 hunks)packages/components/package.json(1 hunks)packages/design-system/.storybook/decorators/index.ts(0 hunks)packages/design-system/.storybook/decorators/withProviders.tsx(0 hunks)packages/design-system/.storybook/preview.ts(1 hunks)packages/design-system/__mocks__/expo-constants.ts(1 hunks)packages/design-system/package.json(1 hunks)packages/design-system/providers/web/SnackbarProvider/__storybook__/stories.tsx(1 hunks)packages/design-system/tests/commom/index.ts(1 hunks)packages/design-system/tests/native/index.ts(1 hunks)packages/design-system/tests/web/utils/__mocks__/theme.ts(1 hunks)packages/design-system/tests/web/utils/index.ts(1 hunks)packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/index.tsx(1 hunks)packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/types.d.ts(1 hunks)packages/design-system/tests/web/utils/providers/ThemeTestProvider/index.tsx(1 hunks)packages/design-system/tests/web/utils/providers/ThemeTestProvider/types.ts(1 hunks)packages/design-system/tests/web/utils/providers/withDesignSystemTestProviders/index.tsx(1 hunks)packages/design-system/tests/web/utils/providers/withDesignSystemTestProviders/types.ts(1 hunks)packages/design-system/tsconfig.build.json(1 hunks)packages/design-system/tsup.config.ts(1 hunks)packages/graphql/index.ts(1 hunks)packages/graphql/package.json(1 hunks)packages/graphql/providers/index.ts(0 hunks)packages/graphql/tests/utils/index.ts(1 hunks)packages/graphql/tests/utils/providers/withGraphqlTestProviders/index.tsx(1 hunks)packages/graphql/tests/utils/providers/withGraphqlTestProviders/types.ts(1 hunks)packages/test/package.json(1 hunks)packages/test/providers/index.ts(1 hunks)packages/test/providers/withStorybookProvidersWrapper/index.tsx(1 hunks)packages/utils/__mocks__/expo-constants.ts(1 hunks)packages/utils/__mocks__/expo-modules-core.ts(1 hunks)packages/utils/__mocks__/expo-secure-store.ts(1 hunks)packages/utils/__mocks__/fetch.ts(1 hunks)packages/utils/__mocks__/file.ts(1 hunks)packages/utils/__mocks__/react-native.ts(1 hunks)packages/utils/__mocks__/style.ts(1 hunks)packages/utils/constants/compose.ts(1 hunks)packages/utils/functions/expo/index.ts(1 hunks)packages/utils/index.ts(1 hunks)packages/wagtail/.storybook/decorators/withProviders.tsx(1 hunks)packages/wagtail/.storybook/preview.ts(2 hunks)packages/wagtail/__mocks__/expo-constants.ts(1 hunks)
💤 Files with no reviewable changes (11)
- packages/components/.storybook/decorators/index.ts
- packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/storybook/stories.tsx
- packages/authentication/modules/profile/index.ts
- packages/graphql/providers/index.ts
- packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/tests/withProviders/types.ts
- packages/components/.storybook/decorators/withProviders.tsx
- packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/tests/withProviders/index.tsx
- packages/components/test_utils/withProviders/types.ts
- packages/design-system/.storybook/decorators/index.ts
- packages/design-system/.storybook/decorators/withProviders.tsx
- packages/components/test_utils/withProviders/index.tsx
🧰 Additional context used
🧬 Code Definitions (11)
packages/graphql/tests/utils/providers/withGraphqlTestProviders/types.ts (1)
packages/graphql/tests/utils/providers/RelayTestProvider/types.ts (1)
RelayTestProviderProps(5-7)
packages/wagtail/.storybook/preview.ts (1)
packages/wagtail/.storybook/decorators/withProviders.tsx (1)
withWagtailProviders(7-7)
packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/types.d.ts (1)
packages/design-system/tests/web/utils/index.ts (1)
ReactSuspenseWithLoadingProps(7-7)
packages/authentication/modules/tests/utils/providers/withAuthenticationTestProviders/types.ts (2)
packages/authentication/modules/tests/utils/index.ts (2)
WithAuthenticationTestProvidersProps(5-5)InitialProfileProviderForTestingProps(2-2)packages/authentication/modules/tests/utils/providers/InitialProfileProviderForTesting/types.ts (1)
InitialProfileProviderForTestingProps(5-8)
packages/design-system/tests/web/utils/providers/withDesignSystemTestProviders/types.ts (2)
packages/design-system/tests/web/utils/providers/ThemeTestProvider/types.ts (1)
ThemeTestProviderProps(5-8)packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/types.d.ts (1)
ReactSuspenseWithLoadingProps(3-6)
packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/index.tsx (2)
packages/design-system/tests/web/utils/index.ts (1)
ReactSuspenseWithLoadingProps(7-7)packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/types.d.ts (1)
ReactSuspenseWithLoadingProps(3-6)
packages/design-system/tests/web/utils/providers/withDesignSystemTestProviders/index.tsx (5)
packages/design-system/tests/web/utils/index.ts (3)
WithDesignSystemProviderProps(10-10)ThemeTestProviderProps(4-4)ReactSuspenseWithLoadingProps(7-7)packages/design-system/tests/web/utils/providers/withDesignSystemTestProviders/types.ts (1)
WithDesignSystemProviderProps(6-9)packages/design-system/tests/web/utils/providers/ThemeTestProvider/types.ts (1)
ThemeTestProviderProps(5-8)packages/utils/index.ts (1)
NotificationProvider(28-28)packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/types.d.ts (1)
ReactSuspenseWithLoadingProps(3-6)
packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/AccountPopover.cy.tsx (1)
packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/__mocks__/profiles.ts (1)
mockUserProfileData(179-179)
packages/wagtail/.storybook/decorators/withProviders.tsx (1)
packages/utils/constants/compose.ts (1)
compose(3-6)
packages/graphql/tests/utils/providers/withGraphqlTestProviders/index.tsx (1)
packages/graphql/tests/utils/providers/withGraphqlTestProviders/types.ts (1)
WithGraphqlTestProvidersProps(5-7)
packages/components/modules/tests/web/utils/withComponentTestProviders/index.tsx (1)
packages/components/modules/tests/web/utils/withComponentTestProviders/types.ts (1)
WithComponentTestProvidersProps(3-5)
🔇 Additional comments (87)
packages/utils/index.ts (1)
1-1: LGTM! New utility export for component compositionThe addition of this export makes the new
composeutility function available throughout the application, which directly supports the PR objective of standardizing providers across testing and Storybook environments.packages/components/baseapp-frontend-imports.d.ts (1)
49-49: Good addition for test provider consistency!Adding this import aligns perfectly with the PR's goal of unifying test and Storybook providers. This makes the web test utilities from the design system package available for auto-imports in the editor, which will help developers use the standardized test providers more easily.
packages/design-system/providers/web/SnackbarProvider/__storybook__/stories.tsx (1)
14-19:❓ Verification inconclusive
Simplification reflects the new unified provider approach.
The removal of the explicit SnackbarProvider wrapper aligns with the PR's goal of using a shared provider wrapper. This simplifies the component and removes duplicated provider setup, assuming the SnackbarProvider is now being applied globally through the new standardized provider setup in Storybook.
🏁 Script executed:
#!/bin/bash # Verify that SnackbarProvider is included in the shared provider setup for Storybook echo "Checking for SnackbarProvider in Storybook setup..." rg -l "SnackbarProvider" --glob "*.{ts,tsx}" packages/*/.storybook/Length of output: 253
Please verify that the global SnackbarProvider is properly configured in your Storybook setup.
The updated code in
packages/design-system/providers/web/SnackbarProvider/__storybook__/stories.tsxremoves the explicit provider wrapper in favor of a unified global provider. However, our automated search couldn’t find any reference to a SnackbarProvider in the Storybook configuration directories (e.g., underpackages/design-system/.storybook/). This might indicate that the global provider setup does not yet include the SnackbarProvider, which could affect how notifications are handled.
- Action Required: Double-check that the shared Storybook provider setup is correctly applying the SnackbarProvider globally.
- Recommendation: If the SnackbarProvider is missing, please update your global provider configuration or adjust the new unified provider approach accordingly.
packages/components/__mocks__/style.ts (1)
1-2: Great modernization to ES Module syntax!Converting from CommonJS to ES Module syntax (
module.exports =toexport * from) standardizes the export patterns across the codebase. This improves code consistency and aligns with modern JavaScript practices.packages/utils/__mocks__/expo-constants.ts (1)
1-2: Good conversion to ES Module syntax!Updating from CommonJS to ES Module syntax (
module.exports =toexport * from) creates consistency across the codebase and follows modern JavaScript best practices. The removal of the.tsextension in the import path is also correct, as it's not needed with ES Module imports.packages/authentication/__mocks__/expo-constants.ts (1)
1-1: Modernized module export syntax.Great job updating from CommonJS to ES modules syntax. This change aligns with modern JavaScript practices and provides better tree-shaking capabilities.
packages/components/__mocks__/fetch.ts (1)
1-1: Modernized module export syntax.Excellent update from CommonJS (
module.exports) to ES modules syntax. This change:
- Creates consistency across the codebase
- Aligns with the PR objective of unifying test providers
- Enables better static analysis and tree-shaking
The removal of the file extension in the import path is also good practice.
packages/wagtail/__mocks__/expo-constants.ts (1)
1-1: Modernized module export syntax.This update to ES modules syntax is consistent with the changes across other packages and helps standardize the codebase.
packages/utils/__mocks__/style.ts (1)
1-1: Modernized module export syntax.Excellent conversion from CommonJS to ES modules. This systematic approach across all packages will make imports more consistent and easier to maintain.
packages/utils/__mocks__/file.ts (1)
1-1: Good migration to ES modules syntax.This change correctly updates the export mechanism from CommonJS to ES module syntax, aligning with modern JavaScript/TypeScript practices. The update improves consistency across the codebase and supports the PR objective of standardizing providers across testing and Storybook environments.
packages/components/__mocks__/expo-secure-store.ts (1)
1-1: Good migration to ES modules syntax.This change correctly updates the export mechanism from CommonJS to ES module syntax, which is a more modern approach. The file extension has also been properly removed from the import path, aligning with best practices where TypeScript can resolve the file without the extension.
packages/utils/__mocks__/fetch.ts (1)
1-1: Good migration to ES modules syntax.This change correctly updates the export mechanism from CommonJS to ES module syntax. The consistent approach across mock files demonstrates a systematic modernization of the codebase, which supports the PR objective of creating unified providers for testing and Storybook environments.
packages/design-system/__mocks__/expo-constants.ts (1)
1-1: Good migration to ES modules syntax.This change correctly updates the export mechanism from CommonJS to ES module syntax. The centralization of mock implementations through re-exports from a common location helps maintain consistency and reduces duplication across the codebase.
packages/design-system/package.json (1)
48-52: LGTM - Appropriate export configuration for test utilitiesThis change adds a new export path for test-related files following the same pattern as other exports in the package. This is necessary to make the test providers and utilities accessible to other packages, aligning with the PR objective of unifying test and Storybook providers.
packages/utils/__mocks__/expo-secure-store.ts (1)
1-1: Modernized export syntax - Good improvementConverting from CommonJS (
module.exports) to ES Module syntax (export * from) improves code consistency and aligns with modern JavaScript practices. The removal of the file extension in the import path is also correct per TypeScript import resolution standards.packages/utils/__mocks__/react-native.ts (1)
1-1: Modernized export syntax - Good improvementConverting from CommonJS (
module.exports) to ES Module syntax (export * from) improves code consistency and aligns with modern JavaScript practices. The removal of the file extension in the import path is also correct per TypeScript import resolution standards.packages/utils/__mocks__/expo-modules-core.ts (1)
1-1: Modernized export syntax - Good improvementConverting from CommonJS (
module.exports) to ES Module syntax (export * from) improves code consistency and aligns with modern JavaScript practices. The removal of the file extension in the import path is also correct per TypeScript import resolution standards.packages/components/__mocks__/expo-modules-core.ts (1)
1-1: Improved module export syntaxThe change from CommonJS
module.exportsto ES moduleexport *syntax is a positive improvement that modernizes the codebase. This aligns with the PR objective of standardizing providers across test utilities and Storybook.packages/components/__mocks__/file.ts (1)
1-1: Good conversion to ES module syntaxUpdating from CommonJS to ES module syntax is a good practice and aligns with modern JavaScript standards. The removal of the file extension in the import path is also appropriate as TypeScript resolves module paths without extensions.
packages/components/__mocks__/react-native.ts (1)
1-1: Consistent export pattern appliedThis change maintains consistency with the other mock files by converting to ES module syntax. This standardization helps with the PR goal of unifying provider patterns across test and Storybook environments.
packages/components/__mocks__/expo-constants.ts (1)
1-1: Standardized export approachThe conversion to ES module syntax completes the standardization pattern across these mock files. This makes the codebase more consistent and maintainable, supporting the PR's objective of standardizing provider implementations.
packages/test/providers/index.ts (1)
3-3: Export added for Storybook providers wrapperThe addition of
withStorybookProvidersWrapperaligns well with the PR objective to standardize providers across test and Storybook environments.packages/test/package.json (1)
22-23: Added Storybook dependencyThe addition of
@storybook/reactas a dependency is appropriate for implementing Storybook provider wrappers as part of unifying test and Storybook providers.packages/authentication/services/user.ts (1)
1-1: Import path updated to be more specificThe import path for
baseAppFetchhas been updated to use a more specific path. This change doesn't affect functionality but improves code organization.This change is consistent with similar updates in
packages/authentication/services/auth.tsas mentioned in the AI summary.packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/__mocks__/profiles.ts (1)
10-10: Replaced dynamic URL with static value for consistent test dataReplacing
faker.internet.url()with a static string'first-profile'for theurlPathproperty makes tests more predictable and less flaky. This change aligns with the PR objective of standardizing test providers and ensuring consistency.packages/graphql/index.ts (1)
3-3: LGTM - Added test utilities exportExporting the test utilities from the package root index file makes them accessible to consumers, supporting the goal of unifying test and Storybook providers across the application.
packages/design-system/tsconfig.build.json (1)
10-10: LGTM - Included test utilities in build processAdding "tests" to the include array ensures that test utilities are compiled and included in the built package, making them available for reuse across the codebase. This is essential for the shared provider approach being implemented.
packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/types.d.ts (1)
1-6: LGTM - Well-defined TypeScript interface for React Suspense wrapperThe interface is well-structured with appropriate React types and optional properties that provide flexibility. This supports the PR goal of creating a unified wrapper for providers in test and Storybook environments.
packages/authentication/index.ts (1)
5-5: LGTM: Enhanced export for test utilitiesThis new export makes the test utilities available from the authentication package, which aligns with the PR's goal of standardizing providers across tests and Storybook environments.
packages/authentication/modules/tests/utils/providers/InitialProfileProviderForTesting/types.ts (2)
3-3: Import path updated correctlyThe import path for MinimalProfile has been updated to reflect the correct relative location.
7-7: LGTM: Added disabled prop for provider controlAdding the optional
disabledproperty allows conditional application of the provider, which is beneficial for testing scenarios where you might want to isolate component behavior from the provider.packages/design-system/tests/web/utils/providers/ThemeTestProvider/types.ts (1)
1-8: Well-structured provider props interfaceThe ThemeTestProviderProps interface is well-designed with:
- PropsWithChildren extension for React component compatibility
- Optional customTheme for theme customization
- Optional disabled flag for conditional rendering
This aligns perfectly with the PR objective of creating a shared wrapper for providers.
packages/authentication/services/auth.ts (1)
1-1: Import path specificity improvedThe import path has been updated to be more specific, which improves maintainability by making dependencies clearer and more explicit. This change is consistent with modern JavaScript best practices for precise imports.
packages/components/modules/tests/web/utils/withComponentTestProviders/types.ts (1)
1-5: Well-structured interface for test providersThe interface definition follows good TypeScript practices with properly typed optional properties. This aligns perfectly with the PR objective of standardizing providers across testing and Storybook environments by allowing QueryClient configuration when needed.
packages/graphql/tests/utils/providers/withGraphqlTestProviders/types.ts (1)
1-7: Good approach with interface extensionThis interface properly extends
RelayTestProviderPropswhile adding Storybook context support. The inheritance structure promotes code reuse and ensures that GraphQL test providers maintain compatibility with both testing and Storybook environments, which aligns with the PR's unification goal.packages/graphql/package.json (1)
23-24: Required dependency for Storybook integrationAdding the Storybook React dependency is necessary to support the new type imports and functionality. This change enables the standardization of providers across testing and Storybook environments as intended in the PR objectives.
packages/authentication/modules/tests/utils/providers/InitialProfileProviderForTesting/index.tsx (1)
5-5: Improved import path clarityThe change from a relative import to a more specific path makes the code more maintainable and less prone to breakage if files are moved around. This type of refactoring supports the PR's goal of standardizing providers by ensuring consistent and reliable imports.
packages/components/modules/notifications/web/NotificationsList/__storybook__/stories.tsx (1)
34-36: Good addition of query name parameter for LoadingNotificationsList story.This addition of the
queryNameparameter helps identify which GraphQL query should be mocked or intercepted for this loading story. This change aligns well with the PR objective of standardizing provider usage across testing and Storybook environments.packages/utils/functions/expo/index.ts (1)
8-18: Good improvement in error handling for expo-constants.The try-catch implementation for dynamically requiring
expo-constantsmakes this utility more resilient in web and Storybook environments. This ensures components can be rendered consistently without crashing when Expo-specific functionality isn't available.packages/design-system/tests/web/utils/providers/withDesignSystemTestProviders/types.ts (1)
1-9: Well-structured interface for design system provider props.The interface correctly extends
PropsWithChildrenand defines the expected properties for theme and loading state management. This provides good type safety for the design system test providers and aligns with the PR goal of creating standardized provider wrappers.packages/components/modules/tests/web/utils/withComponentCompleteTestProviders/index.tsx (1)
1-7: Good imports for unified test providers.The imports provide a comprehensive set of test providers from different packages, which aligns with the PR objective of identifying and unifying all necessary providers for tests and Storybook.
packages/components/.storybook/preview.ts (1)
1-2: Good implementation of the unified provider approachThe changes effectively implement the PR's goal of standardizing providers across testing and Storybook environments. Using the
withStorybookProvidersWrapperto wrapwithComponentCompleteTestProvidersprovides a consistent approach that can be reused across the codebase, ensuring that components are rendered with the same context in both tests and Storybook.Also applies to: 5-5, 25-25
packages/wagtail/.storybook/preview.ts (1)
1-2: Consistent implementation of the unified provider patternThe changes in this file follow the same pattern established in the components package, using
withStorybookProvidersWrapperto wrap the package-specific providers. The use ofwithWagtailProviders(which is a composition ofwithGraphqlTestProvidersandwithDesignSystemTestProviders) demonstrates a modular approach to provider composition, making the code more maintainable and consistent with the standardization goal of this PR.Also applies to: 5-5, 23-23
packages/components/modules/profiles/web/ProfileMembers/__tests__/__utils__/MembersForTesting/index.tsx (1)
2-2: Good adoption of the standardized provider patternThe changes effectively replace the custom provider wrapper with the standardized
withComponentCompleteTestProviders, ensuring that this test utility uses the same provider setup as other components in the codebase. This implementation supports the PR goal of unifying providers across test environments.Also applies to: 8-8
packages/authentication/modules/tests/utils/providers/withAuthenticationTestProviders/types.ts (1)
1-10: Well-designed type interface for authentication providersThis new interface properly defines the props for the authentication test providers, including support for both test and Storybook environments through the
contextprop of typeStoryContext. The interface extendsPropsWithChildrencorrectly to allow wrapping child components, and includes optional configuration for initial profile testing.The type definition enhances type safety and maintainability while supporting the PR's goal of standardizing providers across different environments.
packages/design-system/tests/web/utils/providers/ThemeTestProvider/index.tsx (1)
1-21: Clean and well-structured test provider implementation.This ThemeTestProvider component is well-designed with appropriate defaults and a clean implementation. It properly handles the disabled case and allows for theme customization via props.
packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/index.tsx (1)
1-18: Well-implemented Suspense wrapper for testing.The ReactSuspenseWithLoading component follows good practices by:
- Providing a sensible default fallback (LoadingState)
- Including a disabled option for testing flexibility
- Correctly implementing React.Suspense
This will be useful for handling loading states consistently across tests.
packages/components/modules/tests/web/index.ts (1)
1-3: Clean exports with good organization.This index file correctly exports the test providers and their types, making them easily accessible for tests throughout the codebase. The explicit export of types ensures good TypeScript support for consumers.
packages/components/package.json (1)
63-68: Good addition of test exports to package.json.Adding the test utilities to the package exports makes them available to consumers in a consistent way, aligning with the PR objective of creating shared providers for tests and Storybook.
The export paths follow the same pattern as other exports in the package, maintaining consistency in the codebase.
packages/test/providers/withStorybookProvidersWrapper/index.tsx (1)
1-11: Well-structured and clean HOC implementationThis higher-order component is well-designed for wrapping Storybook stories with test providers. It accepts a provider function and returns a function that applies that provider to a Story, which is exactly what's needed to unify providers between test and Storybook environments.
package.json (1)
23-24: Good dependency versioning practiceChanging from "latest" to specific version ranges (^2.14.1) is a good practice. This prevents unexpected breaking changes from automatic updates while still allowing compatible minor and patch updates, improving build stability and reproducibility.
packages/components/modules/navigations/web/Header/__storybook__/stories.tsx (1)
5-5: Good standardization of theme importsImporting
defaultThemefrom a centralized utility rather than a local mock aligns with the PR objective of unifying providers across test and Storybook environments. This change helps ensure consistent theming across different rendering contexts.packages/components/modules/navigations/web/NavigationLayout/__storybook__/stories.tsx (2)
5-5: Good standardization of theme importsImporting
defaultThemefrom the centralized utils package instead of a local mock ensures consistency with other components and aligns with the PR objective of unifying providers across environments.
32-32: Consistent use of defaultTheme across layout variationsThe implementation correctly applies the standardized defaultTheme across all layout variations (default, mini, horizontal, and centered), maintaining consistent theming while allowing layout-specific customizations.
Also applies to: 96-97, 109-110, 122-123
packages/components/modules/navigations/web/NavMini/__storybook__/stories.tsx (1)
2-2: Import change aligns with provider standardization objectivesThe updated import path for
defaultThemeis part of the broader effort to standardize provider usage across test and Storybook environments. Using the theme from the design system's test utilities instead of a mock ensures consistent theming between tests and stories.packages/authentication/modules/tests/utils/providers/withAuthenticationTestProviders/index.tsx (1)
8-25: Well-structured HOC implementation for authentication test providersThis higher-order component correctly sets up the authentication providers needed for tests. It appropriately retrieves the profile from either context parameters or direct props, providing flexibility for different usage scenarios.
packages/components/modules/tests/web/utils/withComponentTestProviders/index.tsx (2)
7-13: Good configuration for test QueryClientThe default QueryClient is properly configured with retries disabled, which is ideal for testing environments to avoid unnecessary delays or flaky tests.
15-25: Well-implemented HOC with flexibility for custom query clientsThis HOC follows good practices by:
- Using TypeScript generics to maintain type safety
- Providing a default QueryClient while allowing custom overrides
- Properly forwarding props to the wrapped component
The implementation aligns perfectly with the PR goal of standardizing providers across test environments.
packages/design-system/tsup.config.ts (1)
9-11: Essential build config update to support test utilities exportThis change to the build configuration is necessary to make test utilities available for import by other packages. By including the test directories in the build process, you enable the standardization of providers between test and Storybook environments - a core objective of this PR.
The multi-line format also improves readability and makes future path additions easier.
packages/graphql/tests/utils/index.ts (1)
1-5: Clean and efficient export pattern for GraphQL test utilitiesThis new file follows a well-organized pattern by centrally exporting the GraphQL test utilities and their associated types. The implementation aligns well with the PR objective of unifying test and Storybook providers.
packages/components/modules/comments/web/Comments/__tests__/__utils__/CommentsForTesting/index.tsx (2)
5-5: Appropriate replacement of the test provider importThe change from a custom provider to the standardized
withComponentCompleteTestProvidersaligns with the PR objective of unifying test and Storybook providers.
30-30: Consistent update to use the unified test providerThe export statement has been correctly updated to use the new
withComponentCompleteTestProvidershigher-order component, maintaining the same functionality while standardizing the approach.packages/design-system/tests/web/utils/__mocks__/theme.ts (3)
1-1: Improved import specificityThe change from a module-level import to a more specific relative path improves maintainability and makes dependencies clearer.
8-8: Consistent import path updateThe import path has been updated to use relative paths instead of module-level imports, following the same pattern as the other import changes in this file.
10-10: Cleaner export patternThe addition of the
exportkeyword directly to thedefaultThemeconstant declaration simplifies the export pattern, eliminating the need for a separate export statement at the end of the file.packages/components/modules/navigations/web/NavVertical/__storybook__/stories.tsx (1)
2-2: Standardized theme import for StorybookThe import of
defaultThemehas been updated to use the shared utility from the design system's test directory instead of a local mock. This change supports the PR's goal of standardizing providers across test and Storybook environments.packages/graphql/tests/utils/providers/withGraphqlTestProviders/index.tsx (1)
7-32: Good implementation of the GraphQL test providers HOCThe HOC implementation is well-structured, providing a clean way to wrap components with GraphQL test providers. It handles environment creation and operation resolvers appropriately.
packages/authentication/modules/tests/utils/index.ts (1)
1-5: Clean export structure for authentication test utilitiesThe index file properly exports both the components and their corresponding TypeScript types, following best practices for module organization.
packages/design-system/tests/web/utils/providers/withDesignSystemTestProviders/index.tsx (1)
13-33: Well-structured HOC with nested providersThe higher-order component effectively wraps the original component with multiple providers in the correct order, following React's composition pattern.
packages/design-system/.storybook/preview.ts (1)
1-1: Good implementation of unified provider approach for StorybookThe changes correctly implement the PR objective of unifying test and Storybook providers. The new approach uses composition to combine providers through the
withStorybookProvidersWrapperutility.Also applies to: 7-7, 10-10
packages/wagtail/.storybook/decorators/withProviders.tsx (3)
1-5: Good restructuring of imports!The import changes have simplified the code by using higher-level providers from other packages instead of importing and setting up lower-level components directly. This aligns with the PR objective of standardizing providers across test and Storybook environments.
7-7: Well implemented provider composition patternUsing the
composeutility function to combinewithGraphqlTestProvidersandwithDesignSystemTestProvidersis a clean approach that follows functional programming best practices. This simplifies the provider setup and makes it more maintainable.
9-9: Consistent export patternExporting
withWagtailProvidersas the default export maintains the same usage pattern as before while implementing the new standardized approach.packages/components/modules/notifications/web/NotificationsPopover/__tests__/NotificationsPopoverForTesting/index.tsx (3)
1-2: Good import simplificationUsing a centralized import path for test providers improves maintainability and follows the PR's goal of standardizing provider usage. The import is now cleaner and more maintainable.
4-5: Clean implementation of the new provider patternThe implementation clearly applies the centralized test provider to the
NotificationsPopovercomponent, making it consistent with other components in the codebase and aligning with the PR objective to unify provider wrappers.
7-7: Consistent export patternDirectly exporting the wrapped component simplifies the code and maintains a consistent pattern across the codebase.
packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/AccountPopover.cy.tsx (3)
37-42: Good prop structure refactoringThe change from directly passing
initialProfileto using a structuredInitialProfileProviderForTestingPropsobject improves the component API by grouping related props. This makes the component more maintainable as it's clearer which props relate to which provider.
57-61: Consistent prop structure implementationThis change maintains consistency with the updated component API, using the same structured prop pattern established earlier in the file.
129-129: Consistent prop structure applicationThe prop structure change has been consistently applied throughout all test cases, which ensures test reliability and maintainability.
packages/design-system/tests/web/utils/index.ts (5)
1-2: Appropriate use of 'use client' directiveThe
'use client'directive indicates this code is meant to run on the client side in a React Server Components environment, which is correct for UI component testing utilities.
3-4: Good export structure for theme providerCleanly exports both the component and its TypeScript type, which provides good type safety for consumers of this module.
6-7: Well-organized suspense provider exportsFollows the same clear pattern of exporting both the component and its TypeScript type.
9-10: Consistent export pattern for design system providersMaintains the same export pattern, creating a predictable API for consumers.
12-12: Good re-export of theme mocksRe-exporting the theme mocks makes them accessible from this centralized location, reducing import paths throughout the codebase.
packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/AccountPopoverForTesting/index.tsx (3)
2-2: Good addition of the unified test provider package.This new import aligns with the PR objective of unifying providers across testing and Storybook environments by leveraging the centralized
withComponentCompleteTestProvidersutility.
4-4: Implementation successfully refactored to use the unified provider wrapper.The implementation now uses the higher-order component approach with
withComponentCompleteTestProvidersinstead of manually wrapping with providers. This simplifies the code and ensures consistent provider application across tests.
6-6: Export statement appropriately simplified.The export statement has been streamlined to directly export the wrapped component, eliminating the previous dependency on
withProviders. This change completes the refactoring to use the unified provider approach.
anicioalexandre
added
needs changes
priscilladeroode
force-pushed
the
BA-2287-fe-unify-test-and-storybook-providers
branch
from
5d34518 to
6293227
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
packages/wagtail/.storybook/decorators/withProviders.tsx (2)
1-1: Remove unused import FC from ReactThe
FCtype from React is imported but not used anywhere in the file. This unused import should be removed to keep the code clean.-import { FC } from 'react'
1-10: Excellent refactoring of provider implementationThis refactor successfully transforms the complex provider implementation into a simpler, more maintainable composition of reusable providers. The change aligns perfectly with the PR objective of standardizing providers across testing and Storybook environments.
Consider adding a brief comment explaining what these providers do to help future developers understand their purpose and the composition order.
import { withDesignSystemTestProviders } from '@baseapp-frontend/design-system/tests/web/utils' import { withGraphqlTestProviders } from '@baseapp-frontend/graphql' import { compose } from '@baseapp-frontend/utils/dist/constants/compose' +/** + * Combines GraphQL and Design System providers for Wagtail components in Storybook + * Order: Design System providers applied first, then GraphQL providers + */ export const withWagtailProviders = compose(withGraphqlTestProviders, withDesignSystemTestProviders) export default withWagtailProviderspackages/authentication/modules/tests/utils/providers/withAuthenticationTestProviders/index.tsx (4)
8-14: Consider adding JSDoc documentation for this HOC.This Higher-Order Component plays an important role in providing authentication context for tests. Adding JSDoc comments would help other developers understand its purpose, parameters, and usage examples.
+/** + * HOC that wraps components with authentication-related providers for testing purposes. + * Provides Jotai state management and initial profile context. + * + * @template Props - The props type of the wrapped component + * @param {FC<Props>} Component - The component to wrap with authentication providers + * @returns {FC<Props & WithAuthenticationTestProvidersProps>} - The wrapped component with providers + */ const withAuthenticationTestProviders = <Props extends object>(Component: FC<Props>) => ({ context, InitialProfileProviderForTestingProps, ...props }: Props & WithAuthenticationTestProvidersProps) => {
15-17: Add a default fallback for currentProfile.The currentProfile is extracted using optional chaining, but there's no fallback if both sources are undefined. Consider adding a default value to prevent potential issues.
const currentProfile = - context?.parameters?.initialProfile || InitialProfileProviderForTestingProps?.initialProfile + context?.parameters?.initialProfile || InitialProfileProviderForTestingProps?.initialProfile || null
9-14: Consider adding a displayName for debugging purposes.Adding a displayName to the wrapped component would make debugging easier by providing a clear name in React DevTools.
const withAuthenticationTestProviders = <Props extends object>(Component: FC<Props>) => { + const WrappedWithAuthenticationProviders = ( ({ context, InitialProfileProviderForTestingProps, ...props }: Props & WithAuthenticationTestProvidersProps) => { const currentProfile = context?.parameters?.initialProfile || InitialProfileProviderForTestingProps?.initialProfile return ( <JotaiProvider> <InitialProfileProviderForTesting initialProfile={currentProfile}> <Component {...(props as Props)} {...context} /> </InitialProfileProviderForTesting> </JotaiProvider> ) } + WrappedWithAuthenticationProviders.displayName = `withAuthenticationTestProviders(${ + Component.displayName || Component.name || 'Component' + })`; + + return WrappedWithAuthenticationProviders; }
21-21: Type casting could be improved.The
props as Propscast might be unnecessary if the types are properly defined. Consider revisiting the type definitions to make this more type-safe.- <Component {...(props as Props)} {...context} /> + <Component {...props} {...context} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (84)
package.json(1 hunks)packages/authentication/__mocks__/expo-constants.ts(1 hunks)packages/authentication/index.ts(1 hunks)packages/authentication/modules/profile/index.ts(0 hunks)packages/authentication/modules/tests/utils/index.ts(1 hunks)packages/authentication/modules/tests/utils/providers/InitialProfileProviderForTesting/index.tsx(1 hunks)packages/authentication/modules/tests/utils/providers/InitialProfileProviderForTesting/types.ts(1 hunks)packages/authentication/modules/tests/utils/providers/withAuthenticationTestProviders/index.tsx(1 hunks)packages/authentication/modules/tests/utils/providers/withAuthenticationTestProviders/types.ts(1 hunks)packages/authentication/package.json(1 hunks)packages/authentication/services/auth.ts(1 hunks)packages/authentication/services/user.ts(1 hunks)packages/components/.storybook/decorators/index.ts(0 hunks)packages/components/.storybook/decorators/withProviders.tsx(0 hunks)packages/components/.storybook/preview.ts(2 hunks)packages/components/__mocks__/expo-constants.ts(1 hunks)packages/components/__mocks__/expo-modules-core.ts(1 hunks)packages/components/__mocks__/expo-secure-store.ts(1 hunks)packages/components/__mocks__/fetch.ts(1 hunks)packages/components/__mocks__/file.ts(1 hunks)packages/components/__mocks__/react-native.ts(1 hunks)packages/components/__mocks__/style.ts(1 hunks)packages/components/__test_utils__/withProviders/index.tsx(0 hunks)packages/components/__test_utils__/withProviders/types.ts(0 hunks)packages/components/baseapp-frontend-imports.d.ts(1 hunks)packages/components/modules/comments/web/Comments/__tests__/__utils__/CommentsForTesting/index.tsx(2 hunks)packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__storybook__/stories.tsx(0 hunks)packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/AccountPopover.cy.tsx(3 hunks)packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/AccountPopoverForTesting/index.tsx(1 hunks)packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/__mocks__/profiles.ts(1 hunks)packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/withProviders/index.tsx(0 hunks)packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/withProviders/types.ts(0 hunks)packages/components/modules/navigations/web/Header/__storybook__/stories.tsx(1 hunks)packages/components/modules/navigations/web/NavMini/__storybook__/stories.tsx(1 hunks)packages/components/modules/navigations/web/NavVertical/__storybook__/stories.tsx(1 hunks)packages/components/modules/navigations/web/NavigationLayout/__storybook__/stories.tsx(1 hunks)packages/components/modules/notifications/web/NotificationsList/__storybook__/stories.tsx(1 hunks)packages/components/modules/notifications/web/NotificationsPopover/__tests__/NotificationsPopoverForTesting/index.tsx(1 hunks)packages/components/modules/profiles/web/ProfileMembers/__tests__/__utils__/MembersForTesting/index.tsx(1 hunks)packages/components/modules/tests/web/index.ts(1 hunks)packages/components/modules/tests/web/utils/withComponentCompleteTestProviders/index.tsx(1 hunks)packages/components/modules/tests/web/utils/withComponentTestProviders/index.tsx(1 hunks)packages/components/modules/tests/web/utils/withComponentTestProviders/types.ts(1 hunks)packages/components/package.json(1 hunks)packages/design-system/.storybook/decorators/index.ts(0 hunks)packages/design-system/.storybook/decorators/withProviders.tsx(0 hunks)packages/design-system/.storybook/preview.ts(1 hunks)packages/design-system/__mocks__/expo-constants.ts(1 hunks)packages/design-system/package.json(1 hunks)packages/design-system/providers/web/SnackbarProvider/__storybook__/stories.tsx(1 hunks)packages/design-system/tests/commom/index.ts(1 hunks)packages/design-system/tests/native/index.ts(1 hunks)packages/design-system/tests/web/utils/__mocks__/theme.ts(1 hunks)packages/design-system/tests/web/utils/index.ts(1 hunks)packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/index.tsx(1 hunks)packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/types.d.ts(1 hunks)packages/design-system/tests/web/utils/providers/ThemeTestProvider/index.tsx(1 hunks)packages/design-system/tests/web/utils/providers/ThemeTestProvider/types.ts(1 hunks)packages/design-system/tests/web/utils/providers/withDesignSystemTestProviders/index.tsx(1 hunks)packages/design-system/tests/web/utils/providers/withDesignSystemTestProviders/types.ts(1 hunks)packages/design-system/tsconfig.build.json(1 hunks)packages/design-system/tsup.config.ts(1 hunks)packages/graphql/index.ts(1 hunks)packages/graphql/package.json(1 hunks)packages/graphql/providers/index.ts(0 hunks)packages/graphql/tests/utils/index.ts(1 hunks)packages/graphql/tests/utils/providers/withGraphqlTestProviders/index.tsx(1 hunks)packages/graphql/tests/utils/providers/withGraphqlTestProviders/types.ts(1 hunks)packages/test/package.json(1 hunks)packages/test/providers/index.ts(1 hunks)packages/test/providers/withStorybookProvidersWrapper/index.tsx(1 hunks)packages/utils/__mocks__/expo-constants.ts(1 hunks)packages/utils/__mocks__/expo-modules-core.ts(1 hunks)packages/utils/__mocks__/expo-secure-store.ts(1 hunks)packages/utils/__mocks__/fetch.ts(1 hunks)packages/utils/__mocks__/file.ts(1 hunks)packages/utils/__mocks__/react-native.ts(1 hunks)packages/utils/__mocks__/style.ts(1 hunks)packages/utils/constants/compose.ts(1 hunks)packages/utils/functions/expo/index.ts(1 hunks)packages/utils/index.ts(1 hunks)packages/wagtail/.storybook/decorators/withProviders.tsx(1 hunks)packages/wagtail/.storybook/preview.ts(2 hunks)packages/wagtail/__mocks__/expo-constants.ts(1 hunks)
💤 Files with no reviewable changes (11)
- packages/components/.storybook/decorators/index.ts
- packages/authentication/modules/profile/index.ts
- packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/storybook/stories.tsx
- packages/design-system/.storybook/decorators/index.ts
- packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/tests/withProviders/types.ts
- packages/graphql/providers/index.ts
- packages/design-system/.storybook/decorators/withProviders.tsx
- packages/components/test_utils/withProviders/index.tsx
- packages/components/.storybook/decorators/withProviders.tsx
- packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/tests/withProviders/index.tsx
- packages/components/test_utils/withProviders/types.ts
🚧 Files skipped from review as they are similar to previous changes (71)
- packages/design-system/tests/native/index.ts
- packages/test/providers/index.ts
- packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/tests/mocks/profiles.ts
- packages/design-system/tests/commom/index.ts
- packages/authentication/index.ts
- packages/graphql/index.ts
- packages/components/modules/tests/web/utils/withComponentTestProviders/types.ts
- packages/graphql/tests/utils/providers/withGraphqlTestProviders/types.ts
- packages/test/package.json
- packages/authentication/modules/tests/utils/providers/InitialProfileProviderForTesting/index.tsx
- packages/authentication/modules/tests/utils/providers/withAuthenticationTestProviders/types.ts
- packages/components/.storybook/preview.ts
- packages/design-system/tests/web/utils/providers/ThemeTestProvider/types.ts
- packages/utils/constants/compose.ts
- packages/authentication/services/auth.ts
- package.json
- packages/components/mocks/fetch.ts
- packages/utils/mocks/expo-constants.ts
- packages/design-system/tsup.config.ts
- packages/utils/index.ts
- packages/authentication/package.json
- packages/authentication/services/user.ts
- packages/design-system/tests/web/utils/providers/withDesignSystemTestProviders/types.ts
- packages/components/modules/profiles/web/ProfileMembers/tests/utils/MembersForTesting/index.tsx
- packages/design-system/providers/web/SnackbarProvider/storybook/stories.tsx
- packages/design-system/tests/web/utils/providers/ThemeTestProvider/index.tsx
- packages/components/mocks/expo-secure-store.ts
- packages/components/baseapp-frontend-imports.d.ts
- packages/wagtail/mocks/expo-constants.ts
- packages/authentication/modules/tests/utils/providers/InitialProfileProviderForTesting/types.ts
- packages/components/modules/tests/web/index.ts
- packages/utils/mocks/style.ts
- packages/graphql/tests/utils/index.ts
- packages/components/modules/navigations/web/NavigationLayout/storybook/stories.tsx
- packages/utils/mocks/expo-secure-store.ts
- packages/components/mocks/expo-modules-core.ts
- packages/design-system/mocks/expo-constants.ts
- packages/authentication/mocks/expo-constants.ts
- packages/graphql/package.json
- packages/components/package.json
- packages/utils/functions/expo/index.ts
- packages/design-system/.storybook/preview.ts
- packages/design-system/tsconfig.build.json
- packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/index.tsx
- packages/components/modules/tests/web/utils/withComponentTestProviders/index.tsx
- packages/components/mocks/file.ts
- packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/types.d.ts
- packages/components/mocks/expo-constants.ts
- packages/components/modules/comments/web/Comments/tests/utils/CommentsForTesting/index.tsx
- packages/test/providers/withStorybookProvidersWrapper/index.tsx
- packages/components/modules/navigations/web/Header/storybook/stories.tsx
- packages/design-system/tests/web/utils/providers/withDesignSystemTestProviders/index.tsx
- packages/design-system/tests/web/utils/mocks/theme.ts
- packages/components/modules/notifications/web/NotificationsPopover/tests/NotificationsPopoverForTesting/index.tsx
- packages/components/mocks/style.ts
- packages/authentication/modules/tests/utils/index.ts
- packages/design-system/package.json
- packages/components/modules/tests/web/utils/withComponentCompleteTestProviders/index.tsx
- packages/wagtail/.storybook/preview.ts
- packages/utils/mocks/file.ts
- packages/utils/mocks/fetch.ts
- packages/utils/mocks/expo-modules-core.ts
- packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/tests/AccountPopover.cy.tsx
- packages/graphql/tests/utils/providers/withGraphqlTestProviders/index.tsx
- packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/tests/AccountPopoverForTesting/index.tsx
- packages/components/modules/navigations/web/NavMini/storybook/stories.tsx
- packages/components/mocks/react-native.ts
- packages/components/modules/navigations/web/NavVertical/storybook/stories.tsx
- packages/design-system/tests/web/utils/index.ts
- packages/components/modules/notifications/web/NotificationsList/storybook/stories.tsx
- packages/utils/mocks/react-native.ts
🔇 Additional comments (6)
packages/wagtail/.storybook/decorators/withProviders.tsx (3)
3-5: LGTM! Good provider importsThese imports correctly bring in the necessary provider components from the design system and GraphQL packages, along with the compose utility function. This aligns well with the PR objective of standardizing providers across test and Storybook environments.
7-7: Well implemented provider compositionThe composition of providers using the
composeutility creates a clean, maintainable approach to wrapping components with multiple providers. This implementation aligns perfectly with the PR objective of creating a shared wrapper for all required providers.Note that the order of composition matters -
withDesignSystemTestProviderswill be applied first, thenwithGraphqlTestProviders.Is this the intended order of provider application? Please verify that components rendered with this composition receive the providers in the correct order for proper context inheritance.
9-9: LGTM! Clean default exportThe default export makes this composed provider easy to import and use in Storybook configurations.
packages/authentication/modules/tests/utils/providers/withAuthenticationTestProviders/index.tsx (3)
1-6: Good job importing necessary dependencies.The imports are well organized with React imports first, followed by third-party libraries, and then local imports.
1-28: Good implementation of the provider HOC pattern.This implementation aligns well with the PR objective of creating a shared wrapper for providers. The HOC pattern makes it easy to apply these providers consistently across tests and Storybook, which should help standardize component rendering across environments.
18-24:❓ Verification inconclusive
Ensure props are passed correctly to the wrapped component.
The component spreads both
{...props as Props}and{...context}to the wrapped Component. This could lead to prop name collisions if context contains properties that overlap with the Component's expected props.Consider restructuring to be more explicit about which props from context should be passed to the Component. For example:
return ( <JotaiProvider> <InitialProfileProviderForTesting initialProfile={currentProfile}> - <Component {...(props as Props)} {...context} /> + <Component + {...(props as Props)} + // Only spread specific needed props from context, or + // Be more explicit about which context props you need + /> </InitialProfileProviderForTesting> </JotaiProvider> )
🏁 Script executed:
#!/bin/bash # Search for context usage in component props to understand the expected structure rg --type typescript -A 2 -B 2 "context\s*[,}]" --glob "packages/authentication/**/*.tsx"Length of output: 126
#!/bin/bash # Re-run the search using the correct file type (tsx) to detect context usage in spread props. rg --type tsx -A 2 -B 2 "context\s*[,}]" packages/authentication/**/*.tsx
priscilladeroode
force-pushed
the
BA-2287-fe-unify-test-and-storybook-providers
branch
from
6293227 to
8764e32
Compare
priscilladeroode
force-pushed
the
BA-2287-fe-unify-test-and-storybook-providers
branch
from
8764e32 to
84bcb9c
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (13)
packages/wagtail/__mocks__/expo-constants.ts (1)
1-3: Transition from CommonJS to ES Module syntax looks good but consider full ES Module syntaxThe code is transitioning from CommonJS to ES Module syntax, which aligns with the PR's objective of standardizing providers. However, it's using a mixed approach with CommonJS
requirefor importing and ES Moduleexport defaultfor exporting.For complete standardization, consider using full ES Module syntax:
-const Constants = require('@baseapp-frontend/test/__mocks__/expo-constants.ts') +import Constants from '@baseapp-frontend/test/__mocks__/expo-constants.ts' export default ConstantsThis would provide better consistency and compatibility with modern JavaScript/TypeScript practices, especially if other files in the project are moving toward ES Modules.
packages/authentication/__mocks__/style.ts (2)
1-3: Consider using consistent module syntaxWhile the change from CommonJS to ES module exports is good, the file still uses CommonJS-style imports (
require()) mixed with ES module exports (export default). Consider using ES module imports for consistency:-const Style = require('@baseapp-frontend/test/__mocks__/style.ts') +import Style from '@baseapp-frontend/test/__mocks__/style.ts' export default StyleThis would make the module syntax consistent throughout the file and follow modern JavaScript practices.
1-1: Consider removing the file extension from import pathThe import statement includes the
.tsfile extension in the path, which is generally not necessary in TypeScript imports and might cause issues in certain build configurations:-const Style = require('@baseapp-frontend/test/__mocks__/style.ts') +const Style = require('@baseapp-frontend/test/__mocks__/style')This would make the import more resilient to potential file extension changes and align with TypeScript best practices.
packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/index.tsx (2)
3-3: Consider using a more maintainable import path.The deeply nested relative import (
../../../../../components/web/displays) could become fragile if the directory structure changes. Consider using path aliases or a more direct import method if this is a common pattern in your codebase.
6-16: Add JSDoc comments for better documentation.Consider adding JSDoc comments to explain the component's purpose and how to use it, especially since it's going to be shared between test and Storybook environments.
+/** + * A wrapper component that conditionally applies React.Suspense with a loading fallback. + * When disabled is true, children are rendered directly without Suspense. + * + * @param {ReactNode} children - The content to render + * @param {ComponentType} CustomFallback - Optional custom component to show during loading + * @param {boolean} disabled - Whether to disable the Suspense wrapper + */ const ReactSuspenseWithLoading: FC<ReactSuspenseWithLoadingProps> = ({ children, CustomFallback = LoadingState, disabled = false, }) => {packages/utils/functions/compose/index.ts (2)
4-5: Consider improving type safety by using generics.The current implementation uses
anytypes which could lead to type-related issues. Consider using TypeScript generics to provide better type safety while maintaining flexibility:- export const compose = - (...hocs: any[]) => - (Component: FC<any>) => + export const compose = + <P extends object>(...hocs: Array<(Component: FC<any>) => FC<any>>) => + (Component: FC<P>): FC<P> =>
3-6: Add JSDoc comments to document the utility function.Adding JSDoc comments would improve code documentation and provide better IDE intellisense support for developers using this function.
+/** + * Composes multiple higher-order components (HOCs) into a single HOC. + * HOCs are applied from right to left (the rightmost HOC is applied first). + * + * @param hocs - Array of higher-order components to compose + * @returns A single HOC that applies all the input HOCs in sequence + * @example + * // Instead of writing: + * // export default withTheme(withRouter(withAuth(MyComponent))) + * // You can write: + * // export default compose(withTheme, withRouter, withAuth)(MyComponent) + */ export const compose = (...hocs: any[]) => (Component: FC<any>) => hocs.reduceRight((AccumulatedComponent, hoc) => hoc(AccumulatedComponent), Component)packages/components/modules/tests/web/utils/withComponentTestProviders/index.tsx (2)
7-13: Consider isolating the QueryClient between test runs.Creating a static QueryClient instance could potentially cause test state to bleed between tests if they run in parallel. For better test isolation, consider creating a new instance per component render.
-const defaultQueryClient = new QueryClient({ - defaultOptions: { - queries: { - retry: false, - }, - }, -}) const withComponentTestProviders = <Props extends object>(Component: FC<Props>) => (props: Props & WithComponentTestProvidersProps) => { const { queryClient, ...restProps } = props + const defaultQueryClient = new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + })
1-28: Add JSDoc documentation for better code usability.Adding JSDoc comments would improve the understanding of this HOC's purpose and usage, especially since it's part of a shared utility for tests and Storybook.
+/** + * Higher-order component that wraps a component with QueryClientProvider for testing purposes. + * This is part of the unified provider system for tests and Storybook. + * + * @param Component - The React component to wrap + * @returns A new component wrapped with QueryClientProvider + */ const withComponentTestProviders = <Props extends object>(Component: FC<Props>) => (props: Props & WithComponentTestProvidersProps) => {packages/authentication/__mocks__/fetch.ts (1)
1-3: LGTM, but consider using consistent module syntax.The change from CommonJS to ES Module export aligns with the PR objectives of standardizing configurations. However, the code mixes CommonJS (
require) with ES Module syntax (export default). While this works with transpilation, consider using consistent ES Module syntax throughout:import Fetch from '@baseapp-frontend/test/__mocks__/fetch.ts'; export default Fetch;packages/utils/__mocks__/fetch.ts (1)
1-3: LGTM, but consider using consistent module syntax.The restructuring of the fetch mock improves code organization by separating import and export statements. However, like the other mock files, this mixes CommonJS (
require) with ES Module syntax (export default). While functional, consider using consistent ES Module syntax for improved maintainability:import Fetch from '@baseapp-frontend/test/__mocks__/fetch.ts'; export default Fetch;packages/wagtail/__mocks__/expo-secure-store.ts (1)
1-3: Consider using consistent ES Module syntax for both import and export.The file currently uses CommonJS syntax (
require) for importing but ES Module syntax (export default) for exporting. While this mixed approach works in environments that support both syntaxes, it would be more consistent to use ES Module syntax throughout:- const ExpoSecureStore = require('@baseapp-frontend/test/__mocks__/expo-secure-store.ts') + import ExpoSecureStore from '@baseapp-frontend/test/__mocks__/expo-secure-store.ts' export default ExpoSecureStoreThis change would align with the PR's goal of standardizing across the codebase and follow modern JavaScript best practices.
packages/wagtail/.storybook/decorators/withProviders.tsx (1)
1-1: Unused import can be removed.The
FCtype is imported from React but not used in this file. Consider removing it to keep imports clean.-import { FC } from 'react'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (106)
package.json(1 hunks)packages/authentication/CHANGELOG.md(1 hunks)packages/authentication/__mocks__/expo-constants.ts(1 hunks)packages/authentication/__mocks__/expo-modules-core.ts(1 hunks)packages/authentication/__mocks__/expo-secure-store.ts(1 hunks)packages/authentication/__mocks__/fetch.ts(1 hunks)packages/authentication/__mocks__/file.ts(1 hunks)packages/authentication/__mocks__/react-native.ts(1 hunks)packages/authentication/__mocks__/style.ts(1 hunks)packages/authentication/index.ts(1 hunks)packages/authentication/modules/profile/index.ts(0 hunks)packages/authentication/modules/tests/utils/index.ts(1 hunks)packages/authentication/modules/tests/utils/providers/InitialProfileProviderForTesting/index.tsx(1 hunks)packages/authentication/modules/tests/utils/providers/InitialProfileProviderForTesting/types.ts(1 hunks)packages/authentication/modules/tests/utils/providers/withAuthenticationTestProviders/index.tsx(1 hunks)packages/authentication/modules/tests/utils/providers/withAuthenticationTestProviders/types.ts(1 hunks)packages/authentication/package.json(2 hunks)packages/authentication/services/auth.ts(1 hunks)packages/authentication/services/user.ts(1 hunks)packages/components/.storybook/decorators/index.ts(0 hunks)packages/components/.storybook/decorators/withProviders.tsx(0 hunks)packages/components/.storybook/preview.ts(2 hunks)packages/components/CHANGELOG.md(1 hunks)packages/components/__mocks__/expo-constants.ts(1 hunks)packages/components/__mocks__/expo-modules-core.ts(1 hunks)packages/components/__mocks__/expo-secure-store.ts(1 hunks)packages/components/__mocks__/fetch.ts(1 hunks)packages/components/__mocks__/file.ts(1 hunks)packages/components/__mocks__/react-native.ts(1 hunks)packages/components/__mocks__/style.ts(1 hunks)packages/components/__test_utils__/withProviders/index.tsx(0 hunks)packages/components/__test_utils__/withProviders/types.ts(0 hunks)packages/components/baseapp-frontend-imports.d.ts(1 hunks)packages/components/modules/comments/web/Comments/__tests__/__utils__/CommentsForTesting/index.tsx(2 hunks)packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__storybook__/stories.tsx(0 hunks)packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/AccountPopover.cy.tsx(3 hunks)packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/AccountPopoverForTesting/index.tsx(1 hunks)packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/__mocks__/profiles.ts(1 hunks)packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/withProviders/index.tsx(0 hunks)packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/withProviders/types.ts(0 hunks)packages/components/modules/navigations/web/Header/__storybook__/stories.tsx(1 hunks)packages/components/modules/navigations/web/NavMini/__storybook__/stories.tsx(1 hunks)packages/components/modules/navigations/web/NavVertical/__storybook__/stories.tsx(1 hunks)packages/components/modules/navigations/web/NavigationLayout/__storybook__/stories.tsx(1 hunks)packages/components/modules/notifications/web/NotificationsList/__storybook__/stories.tsx(1 hunks)packages/components/modules/notifications/web/NotificationsPopover/__tests__/NotificationsPopoverForTesting/index.tsx(1 hunks)packages/components/modules/profiles/web/ProfileMembers/__tests__/__utils__/MembersForTesting/index.tsx(1 hunks)packages/components/modules/tests/web/index.ts(1 hunks)packages/components/modules/tests/web/utils/withComponentCompleteTestProviders/index.tsx(1 hunks)packages/components/modules/tests/web/utils/withComponentTestProviders/index.tsx(1 hunks)packages/components/modules/tests/web/utils/withComponentTestProviders/types.ts(1 hunks)packages/components/package.json(2 hunks)packages/design-system/.storybook/decorators/index.ts(0 hunks)packages/design-system/.storybook/decorators/withProviders.tsx(0 hunks)packages/design-system/.storybook/preview.ts(1 hunks)packages/design-system/CHANGELOG.md(1 hunks)packages/design-system/__mocks__/expo-constants.ts(1 hunks)packages/design-system/__mocks__/expo-secure-store.ts(1 hunks)packages/design-system/__mocks__/react-native.ts(1 hunks)packages/design-system/package.json(2 hunks)packages/design-system/providers/web/SnackbarProvider/__storybook__/stories.tsx(1 hunks)packages/design-system/tests/commom/index.ts(1 hunks)packages/design-system/tests/native/index.ts(1 hunks)packages/design-system/tests/web/utils/__mocks__/theme.ts(1 hunks)packages/design-system/tests/web/utils/index.ts(1 hunks)packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/index.tsx(1 hunks)packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/types.d.ts(1 hunks)packages/design-system/tests/web/utils/providers/ThemeTestProvider/index.tsx(1 hunks)packages/design-system/tests/web/utils/providers/ThemeTestProvider/types.ts(1 hunks)packages/design-system/tests/web/utils/providers/withDesignSystemTestProviders/index.tsx(1 hunks)packages/design-system/tests/web/utils/providers/withDesignSystemTestProviders/types.ts(1 hunks)packages/design-system/tsconfig.build.json(1 hunks)packages/design-system/tsup.config.ts(1 hunks)packages/graphql/CHANGELOG.md(1 hunks)packages/graphql/index.ts(1 hunks)packages/graphql/package.json(2 hunks)packages/graphql/providers/index.ts(0 hunks)packages/graphql/tests/utils/index.ts(1 hunks)packages/graphql/tests/utils/providers/withGraphqlTestProviders/index.tsx(1 hunks)packages/graphql/tests/utils/providers/withGraphqlTestProviders/types.ts(1 hunks)packages/provider/CHANGELOG.md(1 hunks)packages/provider/package.json(1 hunks)packages/test/CHANGELOG.md(1 hunks)packages/test/package.json(2 hunks)packages/test/providers/index.ts(1 hunks)packages/test/providers/withStorybookProvidersWrapper/index.tsx(1 hunks)packages/utils/CHANGELOG.md(1 hunks)packages/utils/__mocks__/expo-constants.ts(1 hunks)packages/utils/__mocks__/expo-modules-core.ts(1 hunks)packages/utils/__mocks__/expo-secure-store.ts(1 hunks)packages/utils/__mocks__/fetch.ts(1 hunks)packages/utils/__mocks__/file.ts(1 hunks)packages/utils/__mocks__/react-native.ts(1 hunks)packages/utils/__mocks__/style.ts(1 hunks)packages/utils/functions/compose/index.ts(1 hunks)packages/utils/functions/expo/index.ts(1 hunks)packages/utils/index.ts(1 hunks)packages/utils/package.json(1 hunks)packages/wagtail/.storybook/decorators/withProviders.tsx(1 hunks)packages/wagtail/.storybook/preview.ts(2 hunks)packages/wagtail/CHANGELOG.md(1 hunks)packages/wagtail/__mocks__/expo-constants.ts(1 hunks)packages/wagtail/__mocks__/expo-modules-core.ts(1 hunks)packages/wagtail/__mocks__/expo-secure-store.ts(1 hunks)packages/wagtail/__mocks__/react-native.ts(1 hunks)packages/wagtail/package.json(1 hunks)
💤 Files with no reviewable changes (11)
- packages/components/.storybook/decorators/index.ts
- packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/storybook/stories.tsx
- packages/graphql/providers/index.ts
- packages/design-system/.storybook/decorators/withProviders.tsx
- packages/components/test_utils/withProviders/types.ts
- packages/components/.storybook/decorators/withProviders.tsx
- packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/tests/withProviders/types.ts
- packages/components/test_utils/withProviders/index.tsx
- packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/tests/withProviders/index.tsx
- packages/authentication/modules/profile/index.ts
- packages/design-system/.storybook/decorators/index.ts
✅ Files skipped from review due to trivial changes (7)
- packages/wagtail/package.json
- packages/authentication/CHANGELOG.md
- packages/design-system/CHANGELOG.md
- packages/provider/package.json
- packages/utils/package.json
- packages/utils/CHANGELOG.md
- packages/components/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (66)
- packages/design-system/tests/native/index.ts
- packages/test/providers/index.ts
- packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/tests/mocks/profiles.ts
- packages/design-system/tests/commom/index.ts
- packages/authentication/index.ts
- packages/components/modules/profiles/web/ProfileMembers/tests/utils/MembersForTesting/index.tsx
- packages/authentication/modules/tests/utils/providers/withAuthenticationTestProviders/types.ts
- packages/authentication/package.json
- packages/authentication/modules/tests/utils/providers/InitialProfileProviderForTesting/types.ts
- packages/components/mocks/expo-modules-core.ts
- packages/test/package.json
- packages/utils/index.ts
- packages/components/modules/navigations/web/NavigationLayout/storybook/stories.tsx
- packages/components/.storybook/preview.ts
- packages/design-system/providers/web/SnackbarProvider/storybook/stories.tsx
- packages/utils/functions/expo/index.ts
- packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/types.d.ts
- packages/components/modules/notifications/web/NotificationsList/storybook/stories.tsx
- packages/graphql/tests/utils/providers/withGraphqlTestProviders/types.ts
- packages/components/baseapp-frontend-imports.d.ts
- packages/components/modules/tests/web/index.ts
- packages/components/mocks/fetch.ts
- packages/test/providers/withStorybookProvidersWrapper/index.tsx
- packages/design-system/tests/web/utils/providers/withDesignSystemTestProviders/types.ts
- packages/components/modules/tests/web/utils/withComponentTestProviders/types.ts
- packages/authentication/services/auth.ts
- packages/utils/mocks/file.ts
- packages/graphql/index.ts
- packages/design-system/tests/web/utils/providers/ThemeTestProvider/index.tsx
- packages/design-system/mocks/expo-constants.ts
- packages/authentication/modules/tests/utils/index.ts
- packages/design-system/tsup.config.ts
- packages/utils/mocks/expo-modules-core.ts
- packages/authentication/mocks/expo-constants.ts
- packages/authentication/services/user.ts
- package.json
- packages/authentication/modules/tests/utils/providers/withAuthenticationTestProviders/index.tsx
- packages/components/mocks/file.ts
- packages/components/modules/tests/web/utils/withComponentCompleteTestProviders/index.tsx
- packages/graphql/tests/utils/providers/withGraphqlTestProviders/index.tsx
- packages/components/modules/navigations/web/NavMini/storybook/stories.tsx
- packages/utils/mocks/expo-secure-store.ts
- packages/components/modules/comments/web/Comments/tests/utils/CommentsForTesting/index.tsx
- packages/design-system/tests/web/utils/providers/ThemeTestProvider/types.ts
- packages/components/mocks/expo-constants.ts
- packages/design-system/tests/web/utils/mocks/theme.ts
- packages/graphql/tests/utils/index.ts
- packages/design-system/package.json
- packages/wagtail/.storybook/preview.ts
- packages/components/modules/navigations/web/NavVertical/storybook/stories.tsx
- packages/components/modules/notifications/web/NotificationsPopover/tests/NotificationsPopoverForTesting/index.tsx
- packages/components/package.json
- packages/utils/mocks/expo-constants.ts
- packages/authentication/modules/tests/utils/providers/InitialProfileProviderForTesting/index.tsx
- packages/utils/mocks/style.ts
- packages/components/mocks/expo-secure-store.ts
- packages/components/modules/navigations/web/Header/storybook/stories.tsx
- packages/utils/mocks/react-native.ts
- packages/design-system/tests/web/utils/index.ts
- packages/design-system/.storybook/preview.ts
- packages/design-system/tests/web/utils/providers/withDesignSystemTestProviders/index.tsx
- packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/tests/AccountPopoverForTesting/index.tsx
- packages/design-system/tsconfig.build.json
- packages/components/mocks/react-native.ts
- packages/graphql/package.json
- packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/tests/AccountPopover.cy.tsx
🧰 Additional context used
🧬 Code Definitions (2)
packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/index.tsx (2)
packages/design-system/tests/web/utils/index.ts (1)
ReactSuspenseWithLoadingProps(7-7)packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/types.d.ts (1)
ReactSuspenseWithLoadingProps(3-6)
packages/components/modules/tests/web/utils/withComponentTestProviders/index.tsx (1)
packages/components/modules/tests/web/utils/withComponentTestProviders/types.ts (1)
WithComponentTestProvidersProps(3-5)
🔇 Additional comments (21)
packages/test/CHANGELOG.md (1)
3-7: Clear and Consistent Changelog Entry for Version 2.1.2This new entry documents the unification of test and Storybook providers succinctly and in line with the PR objectives. The version bump and concise patch note help maintain transparency with consumers of this package.
packages/wagtail/CHANGELOG.md (1)
3-11: Well-Documented Update in Wagtail Changelog for Version 1.0.29The entry clearly lists the unification of test and Storybook providers along with the updated dependencies, making it easy for users to track changes. It aligns nicely with the overarching effort to streamline provider setups.
packages/provider/CHANGELOG.md (1)
3-9: Provider Package Changelog Update is ConsistentThe version bump to 2.0.13 with an updated dependency on @baseapp-frontend/utils (from 3.1.5 to 3.1.6) is clearly recorded. Since this package is not directly affected by the unification of test and Storybook providers, the entry remains focused and appropriate.
packages/graphql/CHANGELOG.md (1)
3-10: GraphQL Package Changelog Entry Accurately Reflects ChangesThe changelog for version 1.2.7 repeatedly notes the unification of test and Storybook providers and updates the dependency to @baseapp-frontend/utils@3.1.6. This entry is clear, informative, and consistent with the other package updates within this coordinated effort.
packages/authentication/__mocks__/style.ts (1)
1-3: LGTM on updating to ES module exportsThe change from CommonJS
module.exportsto ES moduleexport defaultaligns with the PR objectives of standardizing providers across test and Storybook environments. This syntax modernization is a positive step.packages/design-system/tests/web/utils/providers/ReactSuspenseWithLoading/index.tsx (1)
1-18: Well-structured React Suspense wrapper component.The implementation provides a clean abstraction over React's Suspense with sensible defaults. This component will help standardize loading state handling across both testing and Storybook environments, aligning with the PR's goal of unifying providers.
packages/utils/functions/compose/index.ts (2)
1-6: Solid implementation of a compose utility for HOCs.The implementation correctly uses
reduceRightto apply HOCs from right to left, which is the conventional way HOCs are composed in React.
1-6: This utility aligns well with the PR objective of unifying providers.The
composefunction will help standardize how providers are applied across different environments (tests and Storybook), meeting the PR goal of creating a shared wrapper for all required providers.packages/components/modules/tests/web/utils/withComponentTestProviders/index.tsx (2)
1-5: Good organization of imports.The imports are well-organized with React imports first, followed by external dependencies, and then local imports with appropriate spacing.
15-25: Well-implemented HOC with proper type handling.The higher-order component implementation correctly preserves the original component's props types while adding the additional provider props. The use of generics with
Props extends objectand the proper destructuring of the provider-specific props are good practices.packages/design-system/__mocks__/expo-secure-store.ts (1)
1-3: Approved: Successfully updated to ES Module syntaxThe change from CommonJS to ES Module syntax for exporting the mock is consistent with the PR's goal of standardizing providers across test and Storybook environments.
packages/authentication/__mocks__/expo-secure-store.ts (1)
1-3: Approved: Consistent implementation with design-system packageThis change follows the same pattern as the design-system package, maintaining consistency across packages while updating to ES Module syntax. Good job centralizing the mock implementation.
packages/authentication/__mocks__/expo-modules-core.ts (1)
1-3: Approved: Consistent mock export patternThis update to ES Module syntax follows the consistent pattern applied to other mock files, supporting the PR objective of unifying provider approaches across environments.
packages/authentication/__mocks__/file.ts (1)
1-3: Approved: Standardized export approachThe change completes the pattern of standardizing mock exports to ES Module syntax. This enhancement improves consistency and compatibility across test and Storybook environments.
packages/authentication/__mocks__/react-native.ts (1)
1-3: Improved module export syntax.The change from CommonJS
module.exportsto ES Module syntax with named import and default export aligns with modern JavaScript practices and the standardization effort described in the PR objectives.packages/wagtail/__mocks__/react-native.ts (1)
1-3: Modernized module export pattern.This change is consistent with the transition from CommonJS to ES Module syntax seen across other mock files, supporting the PR's goal of standardizing providers across test and Storybook environments.
packages/wagtail/__mocks__/expo-modules-core.ts (1)
1-3: Unified export pattern with ES Module syntax.The conversion from CommonJS to ES Module syntax aligns with the standardization efforts across the codebase, making the mock imports consistent with modern JavaScript practices.
packages/components/__mocks__/style.ts (1)
1-3: Standardized mock export using ES Module syntax.This change follows the same pattern seen in other mock files, creating consistency in how mocks are exported and imported. The transition to ES Module syntax aligns with modern JavaScript practices.
packages/wagtail/.storybook/decorators/withProviders.tsx (3)
3-5: Good refactoring to reuse existing provider components.The imports bring in reusable provider components from the design system and graphql packages, which aligns with the PR objective of unifying test and Storybook providers.
9-9: Updated default export aligns with new implementation.The default export now uses the new function name, maintaining consistency.
7-7:❓ Verification inconclusive
Excellent use of composition pattern.
Using composition to combine providers is a clean pattern that makes the code more maintainable. This successfully implements the shared wrapper mentioned in the PR objectives.
One recommendation: verify that the order of composition (graphql first, then design system) is correct, as the order can matter depending on provider dependencies.
🏁 Script executed:
#!/bin/bash # Check implementations of both providers to verify correct composition order echo "Checking withGraphqlTestProviders implementation:" fd -t f -e ts -e tsx "withGraphqlTestProviders" --exec cat {} \; echo -e "\nChecking withDesignSystemTestProviders implementation:" fd -t f -e ts -e tsx "withDesignSystemTestProviders" --exec cat {} \;Length of output: 374
Re-check Composition Order for Providers
Excellent application of the composition pattern in combining providers. The use of the shared wrapper in
packages/wagtail/.storybook/decorators/withProviders.tsxcleanly integrates both GraphQL and Design System test providers, aligning well with the PR objectives.However, our initial search for the implementations of
withGraphqlTestProvidersandwithDesignSystemTestProvidersdid not return any content. Please double-check manually that:
- The implementation of
withGraphqlTestProvidersindeed initializes required context before the Design System provider.- The ordering (GraphQL first, then Design System) satisfies the dependency needs internally.
Verifying that the composition order is intentional and based on provider dependencies will ensure maintainability and avoid potential side effects.
Acceptance Criteria
As a developer, I want to standardize the way providers (e.g., theme, context, state management) are applied in both testing and Storybook environments so that I can ensure consistency in how components are rendered across different use cases.
Identify the necessary providers used in tests and Storybook.
Create a shared wrapper that includes all required providers.
Update Storybook configuration to use the shared wrapper for rendering components.
Update test utilities to use the shared wrapper.
Ensure that Storybook stories and tests function correctly with the unified provider setup.
Summary by CodeRabbit
Chores & Dependency Updates
Test & Storybook Enhancements
New Utilities