BA-2012: stable versions #166
Conversation
anicioalexandre
requested review from
Hercilio1,
hpmoreira05 and
lucas-caribe
|
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/components/__mocks__/eventEmitterMock.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/components/.eslintrc.js
WalkthroughThis pull request involves version updates and dependency modifications across multiple packages in the monorepo. The changes primarily focus on updating package versions, standardizing dependency versions from caret ( Changes
Possibly related PRs
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 8b2cfcf and 33176fb72489cf2d62ce79a4ad611eb29aa588c7.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
package.json(1 hunks)packages/authentication/CHANGELOG.md(1 hunks)packages/authentication/package.json(1 hunks)packages/components/CHANGELOG.md(1 hunks)packages/components/package.json(1 hunks)packages/design-system/CHANGELOG.md(1 hunks)packages/design-system/package.json(1 hunks)packages/graphql/CHANGELOG.md(1 hunks)packages/graphql/package.json(1 hunks)packages/provider/CHANGELOG.md(1 hunks)packages/provider/package.json(1 hunks)packages/utils/CHANGELOG.md(1 hunks)packages/utils/package.json(2 hunks)packages/wagtail/CHANGELOG.md(1 hunks)packages/wagtail/package.json(1 hunks)pnpm-workspace.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (12)
- packages/components/package.json
- packages/design-system/package.json
- packages/graphql/package.json
- packages/wagtail/package.json
- packages/authentication/package.json
- packages/provider/package.json
- packages/provider/CHANGELOG.md
- packages/graphql/CHANGELOG.md
- packages/wagtail/CHANGELOG.md
- packages/design-system/CHANGELOG.md
- packages/components/CHANGELOG.md
- pnpm-workspace.yaml
🔇 Additional comments (4)
packages/utils/CHANGELOG.md (2)
3-8: Version bump and dependency stabilization looks good
The changelog entry correctly documents:
- Patch version bump (3.0.5)
- Implementation of stable versions for core dependencies, which aligns with the PR objectives
7-7: Verify the impact of removing events from catalogs
The removal of events from catalogs could be a breaking change depending on its usage. Let's verify if this change impacts any consumers.
packages/utils/package.json (1)
4-4: LGTM: Version bump follows semver
The version bump from 3.0.4 to 3.0.5 follows semantic versioning for patch updates.
packages/authentication/CHANGELOG.md (1)
3-9: LGTM: Changelog update is appropriate
The changelog entry properly documents the dependency update to @baseapp-frontend/utils@3.0.5.
package.json
Outdated
There was a problem hiding this comment.
Revert "latest" versions to fixed versions
Using "latest" for @parcel/packager-ts and @parcel/transformer-typescript-types contradicts the PR objective of implementing fixed versions for stability. The "latest" tag can lead to unexpected breaking changes during builds.
Apply this diff to maintain fixed versions:
- "@parcel/packager-ts": "latest",
- "@parcel/transformer-typescript-types": "latest",
+ "@parcel/packager-ts": "2.13.2",
+ "@parcel/transformer-typescript-types": "2.13.2",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "@parcel/packager-ts": "latest", | |
| "@parcel/transformer-typescript-types": "latest", | |
| "@parcel/packager-ts": "2.13.2", | |
| "@parcel/transformer-typescript-types": "2.13.2", |
packages/utils/package.json
Outdated
There was a problem hiding this comment.
Use fixed version instead of caret range
Using ^3.3.0 for events allows minor version updates, which contradicts the PR objective of implementing fixed versions for stability.
Apply this diff to use a fixed version:
- "events": "^3.3.0",
+ "events": "3.3.0",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "events": "^3.3.0", | |
| "events": "3.3.0", |
anicioalexandre
force-pushed
the
BA-2012-stable-versions
branch
from
33176fb to
f2f900b
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
packages/wagtail/.storybook/__mocks__/eventEmitterMock.ts (1)
Line range hint
1-4: Replace usage ofFunctionwith an explicit function type
Static analysis reports that'Function'is a banned type because it is overly broad. Define a specialized type for listener callbacks instead. For example:-export class EventEmitter { - private events: Record<string, Function[]> = {} +type EventListener = (...args: any[]) => void; +export class EventEmitter { + private events: Record<string, EventListener[]> = {} on(event: string, listener: Function) { ...This refactor improves readability, type safety, and aligns with best practices.
🧰 Tools
🪛 Biome (1.9.4)
[error] 17-17: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
packages/components/__mocks__/eventEmitterMock.ts (1)
Line range hint
1-4: Eliminate banned'Function'type usage
According to linting rules,'Function'is overly permissive. Replace it with an explicit function type to improve maintainability and correctness. For instance:-export class EventEmitter { - private events: Record<string, Function[]> = {} +type EventListener = (...args: any[]) => void; +export class EventEmitter { + private events: Record<string, EventListener[]> = {} on(event: string, listener: Function) { ...This change aligns with recommended TypeScript best practices.
🧰 Tools
🪛 Biome (1.9.4)
[error] 17-17: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
🧹 Nitpick comments (4)
packages/design-system/components/inputs/Searchbar/__storybook__/stories.tsx (1)
32-33: No functional changes, but watch out for trailing spaces.These lines are blank or contain closing braces. Double-check for any stray whitespace to keep your file tidy.
packages/design-system/components/avatars/ClickableAvatar/__storybook__/stories.tsx (1)
10-11: Maintain consistency in starry lines.Ensuring that your story configuration lines remain minimal and straightforward helps avoid confusion when reading through the metadata.
packages/components/modules/navigations/Header/AccountMenu/AccountPopover/__storybook__/stories.tsx (1)
Line range hint
19-42: Consider specifying the type parameter for stronger type safety.Currently,
metais typed asMetawithout a type parameter. IfAccountPopoverhas its own props interface, you can do something like:-const meta: Meta = { +const meta: Meta<AccountPopoverProps> = {This ensures stricter type checking for the component’s props in the Storybook context.
packages/components/tsconfig.json (1)
6-8: Consider more robust alternatives to path mappingWhile path mapping works, it's fragile and could break if the
node_modulesstructure changes. Consider these more robust alternatives:
- Use package.json resolutions (Yarn) or overrides (npm) to enforce a single version
- Leverage workspace features for better dependency management
- Properly declare peer dependencies
Example package.json configuration:
{ "resolutions": { "@mui/system": "<version>" } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 33176fb72489cf2d62ce79a4ad611eb29aa588c7 and f2f900b.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (36)
package.json(1 hunks)packages/authentication/CHANGELOG.md(1 hunks)packages/authentication/package.json(1 hunks)packages/components/CHANGELOG.md(1 hunks)packages/components/__mocks__/eventEmitterMock.ts(1 hunks)packages/components/modules/navigations/Header/AccountMenu/AccountPopover/__storybook__/stories.tsx(2 hunks)packages/components/package.json(2 hunks)packages/components/tsconfig.json(1 hunks)packages/design-system/CHANGELOG.md(1 hunks)packages/design-system/components/Logo/__storybook__/stories.tsx(1 hunks)packages/design-system/components/Popover/__storybook__/stories.tsx(1 hunks)packages/design-system/components/Scrollbar/__storybook__/stories.tsx(1 hunks)packages/design-system/components/avatars/AvatarWithPlaceholder/__storybook__/stories.tsx(1 hunks)packages/design-system/components/avatars/ClickableAvatar/__storybook__/stories.tsx(1 hunks)packages/design-system/components/buttons/IconButton/__storybook__/stories.tsx(1 hunks)packages/design-system/components/dialogs/ConfirmDialog/__storybook__/stories.tsx(1 hunks)packages/design-system/components/dialogs/Dialog/__storybook__/stories.tsx(1 hunks)packages/design-system/components/displays/LoadingState/__storybook__/stories.tsx(1 hunks)packages/design-system/components/drawers/SwipeableDrawer/__storybook__/stories.tsx(1 hunks)packages/design-system/components/inputs/Searchbar/__storybook__/stories.tsx(2 hunks)packages/design-system/components/inputs/SocialTextField/__storybook__/stories.tsx(1 hunks)packages/design-system/components/inputs/TextField/__storybook__/stories.tsx(1 hunks)packages/design-system/components/inputs/TextareaField/__storybook__/stories.tsx(1 hunks)packages/design-system/components/typographies/TypographyWithEllipsis/__storybook__/stories.tsx(1 hunks)packages/design-system/package.json(1 hunks)packages/graphql/CHANGELOG.md(1 hunks)packages/graphql/package.json(1 hunks)packages/provider/CHANGELOG.md(1 hunks)packages/provider/package.json(1 hunks)packages/utils/CHANGELOG.md(1 hunks)packages/utils/package.json(2 hunks)packages/wagtail/.storybook/__mocks__/eventEmitterMock.ts(1 hunks)packages/wagtail/CHANGELOG.md(1 hunks)packages/wagtail/package.json(1 hunks)packages/wagtail/tsconfig.json(1 hunks)pnpm-workspace.yaml(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/design-system/components/displays/LoadingState/storybook/stories.tsx
🚧 Files skipped from review as they are similar to previous changes (16)
- packages/design-system/package.json
- packages/graphql/package.json
- packages/authentication/package.json
- packages/wagtail/package.json
- packages/provider/package.json
- packages/utils/package.json
- packages/wagtail/CHANGELOG.md
- packages/design-system/CHANGELOG.md
- packages/utils/CHANGELOG.md
- package.json
- packages/provider/CHANGELOG.md
- packages/components/package.json
- packages/graphql/CHANGELOG.md
- packages/components/CHANGELOG.md
- packages/authentication/CHANGELOG.md
- pnpm-workspace.yaml
🧰 Additional context used
🪛 Biome (1.9.4)
packages/wagtail/.storybook/__mocks__/eventEmitterMock.ts
[error] 17-17: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
packages/components/__mocks__/eventEmitterMock.ts
[error] 17-17: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
🔇 Additional comments (41)
packages/wagtail/tsconfig.json (1)
18-18: LGTM! Excluding the dist directory follows TypeScript best practices.
The addition of "dist" to the exclude array prevents TypeScript from processing compiled files, which is the correct approach to avoid potential circular compilation issues and improve build performance.
Let's verify if this configuration is consistent across other packages in the monorepo:
✅ Verification successful
Consistent exclude patterns found across packages with minor variations
The verification shows that all packages consistently exclude both "node_modules" and "dist" directories, with some packages having additional excludes for their specific needs:
- Most packages follow the basic pattern of excluding "node_modules" and "dist"
- graphql and design-system packages additionally exclude "./relay.config.js"
- components package has extra excludes for test files ("jest", "/*.test.ts", "/*.test.tsx")
These variations are justified by the specific requirements of each package, and the core exclusion of "dist" is consistently implemented across all packages.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check tsconfig.json files across packages for consistent exclude patterns
# Expected: All tsconfig.json files should exclude both node_modules and dist
# Find all tsconfig.json files in packages directory
fd tsconfig.json packages/ --exec sh -c '
echo "=== Checking $1 ==="
# Extract and display the exclude array
jq -r ".exclude // [] | @csv" "$1"
echo
' sh {}
Length of output: 911
packages/design-system/components/inputs/TextField/__storybook__/stories.tsx (2)
6-9: Good adherence to modern Storybook patterns.
Switching to a meta constant is a more explicit way of defining Storybook metadata while improving type safety. No issues found with these lines.
11-11: Exporting the meta object is clear and conventional.
This standardized export style aligns well with current best practices. No further changes needed.
packages/design-system/components/inputs/TextareaField/__storybook__/stories.tsx (2)
6-9: Clearer separation of metadata definition.
Declaring the meta object inline with its title and component references is a neat approach that improves readability.
11-11: Consistent export of the meta object.
This export approach matches the project’s new Storybook structure. Looks good.
packages/design-system/components/Logo/__storybook__/stories.tsx (3)
1-1: Importing ComponentType is appropriate for strong typing.
This addition ensures the Logo component is typed properly when cast. Nice detail.
9-12: Properly typed Storybook meta object.
Casting Logo as ComponentType<LogoProps> further clarifies the component’s type in Storybook. Excellent usage of generics.
14-14: Clean export of the meta object.
This aligns with the updated Storybook conventions. No further adjustments needed.
packages/design-system/components/inputs/SocialTextField/__storybook__/stories.tsx (2)
6-9: Clear definition of the Storybook meta.
Defining the metadata constant separately from the export is a neat and type-safe approach.
11-11: Straightforward export aligns with best practices.
Consistent use of meta for the default export is good for maintainability and clarity.
packages/design-system/components/typographies/TypographyWithEllipsis/__storybook__/stories.tsx (2)
6-9: Consistent implementation of Storybook's Meta object.
This approach of defining a meta constant and then exporting it enhances clarity and aligns with recommended Storybook patterns.
11-11: Exporting meta as default is a recommended best practice.
Exporting the meta constant helps keep the Storybook configuration reusable and consistent across components.
packages/design-system/components/buttons/IconButton/__storybook__/stories.tsx (2)
8-11: Refactored default export to a dedicated meta constant.
This refactoring is in line with modern Storybook guidelines, making the configuration explicit and easily maintained.
13-13: Default export of meta ensures clear structure.
Keeping all Storybook metadata in a single constant fosters consistency and readability for future updates.
packages/design-system/components/Scrollbar/__storybook__/stories.tsx (2)
7-10: Refined Storybook configuration via a meta constant.
Defining metadata in a dedicated constant clarifies the component's configuration and ensures consistent typing.
12-12: Default export of the meta object.
Aligns with the recommended practice for exporting Storybook metadata, making it easier for future maintenance.
packages/design-system/components/avatars/AvatarWithPlaceholder/__storybook__/stories.tsx (2)
7-10: Adoption of a dedicated meta constant.
Establishes a uniform pattern across Storybook stories, improving readability and type safety.
12-12: Exporting meta as default.
This standard approach makes the story definition more explicit and consistent with other components.
packages/design-system/components/inputs/Searchbar/__storybook__/stories.tsx (3)
1-2: Good use of a typed import from the updated utils package.
By importing WithControllerProps, you ensure that the SearchbarProps are augmented with controller-related properties, which enhances clarity and type-safety.
8-8: Appropriate use of generics for Storybook metadata.
Defining meta with Meta<WithControllerProps<SearchbarProps>> accurately reflects the expanded prop structure. This improves maintainability and ensures type hints are preserved in Storybook.
34-34: Consistent default export pattern.
Switching to export default meta aligns with Storybook’s recommended approach to define and export story metadata.
packages/design-system/components/drawers/SwipeableDrawer/__storybook__/stories.tsx (3)
9-9: Clearer Storybook setup with typed metadata.
Defining meta as Meta<SwipeableDrawerProps> helps ensure that the SwipeableDrawer stories are properly typed and documented.
12-13: Logical grouping of metadata properties.
Keeping title and component together inside meta increases readability and consistency in your Storybook configuration.
14-14: Exporting metadata as default is consistent with recommended patterns.
Refactoring to export default meta follows the latest Storybook best practices.
packages/design-system/components/dialogs/ConfirmDialog/__storybook__/stories.tsx (3)
9-9: Meta constant approach for story typing.
Typing the metadata as Meta<ConfirmDialogProps> improves clarity around the story’s properties.
12-13: Structured metadata definition.
Organizing configuration details for Storybook in a single object fosters maintainability.
14-14: Consistent default export pattern.
Exporting meta as default aligns with the overall refactoring across your design system to improve type coverage and consistency.
packages/design-system/components/avatars/ClickableAvatar/__storybook__/stories.tsx (2)
7-7: Clearer type definition for the avatar props.
By specifying Meta<ClickableAvatarProps>, you are providing precise typing and better auto-documentation support in Storybook.
12-12: Default export maintains a consistent style across the codebase.
Defining the meta constant and exporting it simplifies the story setup and improves readability.
packages/components/modules/navigations/Header/AccountMenu/AccountPopover/__storybook__/stories.tsx (1)
44-44: No issues with the default export structure.
The default export is consistent with the updated approach and aligns with other stories in the repository.
packages/design-system/components/Popover/__storybook__/stories.tsx (2)
9-12: Good practice specifying the type parameter.
Defining meta as Meta<MenuPopoverProps> ensures better type checking for the Popover's props.
14-14: Default export is concise and clear.
Exporting meta as the default is a straightforward approach, improving clarity in the Storybook configuration.
packages/design-system/components/dialogs/Dialog/__storybook__/stories.tsx (2)
9-12: Clear type specification on meta.
Defining meta as Meta<DialogProps> helps maintain better type safety and consistency in Storybook stories.
14-14: Maintains alignment with the updated Storybook export style.
No issues found with the default export. The structure follows the recommended pattern.
packages/wagtail/.storybook/__mocks__/eventEmitterMock.ts (3)
8-8: Optional chaining implemented correctly
Utilizing optional chaining (?.) here is a safe way to avoid errors when events[event] might be undefined. Good job on preventing potential runtime exceptions.
13-13: Optional chaining in emit()
Again, optional chaining is properly utilized. This ensures that the iteration only executes if the array is defined.
19-20: Confirm necessity of // @ts-ignore
While this ignore directive prevents type errors, consider whether it's strictly necessary. If the filter logic can be safely typed (e.g., when you change the function type away from Function), this directive may no longer be required.
packages/components/__mocks__/eventEmitterMock.ts (3)
8-8: Optional chaining for safeguarding your push() operation
This usage of ?.push() is a robust way to avoid potential errors when events[event] is uninitialized.
13-13: Safe iteration with optional chaining
Using ?.forEach() ensures that no error is thrown if this.events[event] is undefined. This helps prevent runtime exceptions.
19-20: Assess // @ts-ignore
If you switch from using Function to a strongly typed function signature, you may not require this ignore directive. Consider removing it after applying the stronger type.
packages/components/tsconfig.json (1)
6-8: Verify if this path mapping is necessary
The explicit path mapping to node_modules suggests potential dependency resolution issues. Let's verify if this is required:
✅ Verification successful
Let me try a different approach to verify the necessity of this path mapping.
Let me try one more verification to check the actual imports and usage patterns.
Based on the verification results, I can now provide a final response:
Path mapping to @mui/system is necessary for consistent dependency resolution
The path mapping in tsconfig.json is justified based on the following findings:
- Multiple packages (wagtail, design-system, components) depend on @mui/system
- The codebase has direct imports of @mui/system in several files
- All Material-UI dependencies are managed through a "catalog:material-ui" version specification
- The path mapping ensures consistent resolution of @mui/system across the monorepo
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for multiple @mui/system versions that might cause conflicts
echo "Checking for multiple @mui/system installations..."
find . -name "node_modules" -type d -exec find {} -name "@mui" -type d \; | grep system
# Check package-lock.json or yarn.lock for @mui/system references
echo "Checking lock files for @mui/system versions..."
find . -name "package-lock.json" -o -name "yarn.lock" -exec grep -l "@mui/system" {} \;
Length of output: 391
Script:
#!/bin/bash
# Check package.json files for @mui/system dependency
echo "Checking package.json files for @mui/system..."
fd package.json --type f --exec grep -l "@mui/system" {}
# Check for imports of @mui/system in the codebase
echo "Checking imports of @mui/system..."
rg "from ['\"]\@mui/system['\"]" -l
# Check tsconfig files for any other path mappings
echo "Checking other tsconfig files..."
fd tsconfig.json --type f --exec cat {}
Length of output: 4764
Script:
#!/bin/bash
# Check for @mui/system imports with proper escaping
echo "Checking @mui/system imports..."
rg -l "from ['\"]@mui/system['\"]"
# Check package.json contents for dependency details
echo "Checking package.json contents for @mui/system..."
fd package.json --type f --exec sh -c 'echo "=== $1 ==="; cat "$1"' sh {}
# Look for any Material-UI related imports
echo "Checking Material-UI related imports..."
rg -l "@mui/"
Length of output: 47996
baseapp-frontendecosystem (template and packages) aiming for more stability on the core dependencies we useSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
package.jsonfiles.