fix: cleanup and using mobx-react-lite #356
Conversation
WalkthroughThis pull request introduces a series of consistent changes across multiple packages in the contact center project. The modifications primarily focus on updating build scripts to include a clean step before webpack, replacing the Changes
Possibly related PRs
Poem
Finishing Touches
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 (
|
rarajes2
changed the title
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
| @@ -18,7 +18,7 @@ | |||
| "clean": "rm -rf dist && rm -rf node_modules", | |||
| "clean:dist": "rm -rf dist", | |||
| "build": "yarn run -T tsc", | |||
There was a problem hiding this comment.
Are we still using this build command?
There was a problem hiding this comment.
Not using but keeping it if we need to test something. If we don't use it for quite some time, we can remove this.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/contact-center/task/webpack.config.js (1)
12-16: LGTM! Consider adding MobX to externals.The externals configuration is a good optimization to reduce bundle size and avoid dependency duplication. Since this PR involves MobX-related changes, consider adding MobX as an external as well.
externals: { react: 'react', 'react-dom': 'react-dom', '@webex/cc-store': '@webex/cc-store', + 'mobx-react-lite': 'mobx-react-lite', + 'mobx': 'mobx' }packages/contact-center/task/src/IncomingTask/index.tsx (1)
2-2: LGTM! Consider using mobx-react-lite's built-in types.The migration is correct. For better type safety, consider using the Observer type from mobx-react-lite.
-const IncomingTask: React.FunctionComponent<IncomingTaskProps> = observer(({onAccepted, onDeclined}) => { +import type {Observer} from 'mobx-react-lite'; + +const IncomingTask: Observer<IncomingTaskProps> = ({onAccepted, onDeclined}) => {packages/contact-center/station-login/src/station-login/index.tsx (1)
3-3: LGTM! Consider using mobx-react-lite's built-in types.The migration is correct. For better type safety, consider using the Observer type from mobx-react-lite.
-const StationLogin: React.FunctionComponent<StationLoginProps> = observer(({onLogin, onLogout}) => { +import type {Observer} from 'mobx-react-lite'; + +const StationLogin: Observer<StationLoginProps> = ({onLogin, onLogout}) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
package.json(1 hunks)packages/contact-center/cc-widgets/package.json(2 hunks)packages/contact-center/station-login/package.json(2 hunks)packages/contact-center/station-login/src/station-login/index.tsx(1 hunks)packages/contact-center/store/package.json(1 hunks)packages/contact-center/task/package.json(2 hunks)packages/contact-center/task/src/IncomingTask/index.tsx(1 hunks)packages/contact-center/task/src/TaskList/index.tsx(1 hunks)packages/contact-center/task/webpack.config.js(1 hunks)packages/contact-center/user-state/package.json(1 hunks)packages/contact-center/user-state/src/user-state/index.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🧰 Additional context used
📓 Learnings (1)
packages/contact-center/task/webpack.config.js (1)
Learnt from: Shreyas281299
PR: webex/widgets#345
File: packages/contact-center/cc-widgets/webpack.config.js:6-12
Timestamp: 2024-12-04T07:40:14.028Z
Learning: In the webex-widgets project, optimizations such as reducing bundle size and improving webpack configurations in files like `packages/contact-center/cc-widgets/webpack.config.js` are handled in separate tickets.
🔇 Additional comments (15)
packages/contact-center/task/src/TaskList/index.tsx (1)
3-3: LGTM! Verify mobx-react-lite compatibility.The migration to
mobx-react-liteis appropriate as this is a functional component. The implementation remains compatible sincemobx-react-liteis specifically optimized for function components.Let's verify no class components are using mobx-react-lite:
✅ Verification successful
✓ Verified: mobx-react-lite usage is correct
The migration to
mobx-react-liteis appropriate as TaskList is implemented as a functional component using hooks. No class components were found in the contact-center package that could cause compatibility issues.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for potential class components using mobx-react-lite ast-grep --pattern 'class $_ extends React.Component { $$$ }' # Search for mobx-react-lite imports rg "from 'mobx-react-lite'" -lLength of output: 65
Script:
#!/bin/bash # Search for class components with different patterns ast-grep --pattern 'class $_ extends Component { $$$ }' ast-grep --pattern 'class $_ extends PureComponent { $$$ }' # Search for mobx-react and mobx-react-lite imports rg "from ['\"]mobx-react['\"]" -l rg "from ['\"]mobx-react-lite['\"]" -l # Search for observer usage patterns rg "observer\((.*?)\)" --multilineLength of output: 4072
Script:
#!/bin/bash # Search for mobx imports in contact-center package rg "from ['\"]mobx-react(?:-lite)?['\"]" packages/contact-center -l # Check TaskList component structure cat packages/contact-center/task/src/TaskList/index.tsx # Search for other potential class components in contact-center ast-grep --pattern 'class $_ extends $_ { $$$ }' packages/contact-centerLength of output: 888
packages/contact-center/user-state/src/user-state/index.tsx (1)
3-3: LGTM! Clean migration to mobx-react-lite.The migration is correct as this component is functional and properly typed with TypeScript interfaces.
packages/contact-center/store/package.json (1)
18-18: LGTM! Clean build step added.Adding the clean step before webpack ensures no stale artifacts remain from previous builds.
packages/contact-center/user-state/package.json (2)
17-17: LGTM! Clean build step added.Adding the clean step before webpack ensures no stale artifacts remain from previous builds.
23-23: LGTM! Appropriate dependency update.Replacing mobx-react with mobx-react-lite is a good optimization since the project exclusively uses functional components.
packages/contact-center/station-login/package.json (3)
17-17: LGTM! Clean build step added.Adding the clean step before webpack ensures no stale artifacts remain from previous builds.
23-23: LGTM! Appropriate dependency update.Replacing mobx-react with mobx-react-lite is a good optimization since the project exclusively uses functional components.
57-59: LGTM! Jest configuration enhanced.Added moduleNameMapper to handle CSS/LESS/SCSS imports in tests, which helps fix broken unit tests.
packages/contact-center/task/package.json (5)
14-15: LGTM! Added clean scripts.New clean scripts provide flexibility for cleaning dist directory or complete cleanup including node_modules.
17-17: LGTM! Clean build step added.Adding the clean step before webpack ensures no stale artifacts remain from previous builds.
23-23: LGTM! Appropriate dependency update.Replacing mobx-react with mobx-react-lite is a good optimization since the project exclusively uses functional components.
47-50: LGTM! Proper dependency organization.Moving React and React DOM to peerDependencies is the correct approach for a library package.
57-59: LGTM! Jest configuration enhanced.Added moduleNameMapper to handle CSS/LESS/SCSS imports in tests, which helps fix broken unit tests.
packages/contact-center/cc-widgets/package.json (2)
21-21: LGTM! Clean build step added.Adding the clean step before webpack ensures no stale artifacts remain from previous builds.
69-71: LGTM! Jest configuration enhanced.Added moduleNameMapper to handle CSS/LESS/SCSS imports in tests, which helps fix broken unit tests.
| externals: { | ||
| react: 'react', | ||
| 'react-dom': 'react-dom', | ||
| '@webex/cc-store': '@webex/cc-store', | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Add missing peer dependencies for webpack externals
The following externals need to be added as peer dependencies in package.json:
- react
- react-dom
- @webex/cc-store
🔗 Analysis chain
Verify peer dependencies match externals configuration.
Ensure that the package.json peer dependencies align with the webpack externals configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if package.json peer dependencies match webpack externals
# Get peer dependencies from package.json
echo "Checking peer dependencies in package.json:"
cat package.json | jq '.peerDependencies'
# Verify that all externals are listed as peer dependencies
echo -e "\nChecking for potential mismatches between externals and peer dependencies:"
for dep in "react" "react-dom" "@webex/cc-store"; do
if ! cat package.json | jq -e ".peerDependencies.\"$dep\""; then
echo "Warning: External '$dep' is not listed in peer dependencies"
fi
done
Length of output: 1094
rarajes2
changed the title
We needed to update the jest config |
|
🎉 This PR is included in version 1.28.0-ccwidgets.8 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
# [1.28.0-ccwidgets.8](webex/widgets@v1.28.0-ccwidgets.7...v1.28.0-ccwidgets.8) (2025-01-10) ### Bug Fixes * **bundling:** fixed the multiple React instance and useRef errors ([webex#355](webex#355)) ([473cd4f](webex@473cd4f)) * cleanup and using mobx-react-lite ([webex#356](webex#356)) ([0b304c4](webex@0b304c4))
|
🎉 This PR is included in version 1.28.0-ccconnectors.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
# [1.28.0-ccconnectors.1](webex/widgets@v1.27.5...v1.28.0-ccconnectors.1) (2025-02-05) ### Bug Fixes * add material to all components ([webex#376](webex#376)) ([de0ca28](webex@de0ca28)) * **bundling:** fixed the multiple React instance and useRef errors ([webex#355](webex#355)) ([473cd4f](webex@473cd4f)) * **call-control:** add-call-control-widget ([webex#362](webex#362)) ([a677f5e](webex@a677f5e)) * **cc-station-login:** material design ui ([webex#377](webex#377)) ([aec7034](webex@aec7034)) * **cc-store:** receive webex on init, add store types ([webex#341](webex#341)) ([9648969](webex@9648969)) * **cc-widgets:** ship-all-widgets-together ([webex#345](webex#345)) ([83d5a37](webex@83d5a37)) * cleanup and using mobx-react-lite ([webex#356](webex#356)) ([0b304c4](webex@0b304c4)) * convert npm to yarn ([webex#234](webex#234)) ([432ed3c](webex@432ed3c)) * **release:** add-publish-step-in-tooling ([webex#334](webex#334)) ([ca32235](webex@ca32235)) * rename agent state to user state ([webex#361](webex#361)) ([fe409db](webex@fe409db)) * **samples:** change samples index html hrefs ([webex#367](webex#367)) ([ff126ab](webex@ff126ab)) * **user-state:** receive agent stateChange event ([webex#350](webex#350)) ([21d6ce7](webex@21d6ce7)) ### Features * **cc-components:** setup and move user state sample ui comp ([webex#359](webex#359)) ([16a44d0](webex@16a44d0)) * **cc-store:** add logger from sdk ([webex#354](webex#354)) ([a62494b](webex@a62494b)) * **cc-widgets:** added Agent-Multi-Login-Alert Feature ([webex#364](webex#364)) ([f7d75ca](webex@f7d75ca)) * **release:** add new branch to circleci ([18f7bec](webex@18f7bec)) * **release:** publish pipeline for wxcc widgets ([webex#324](webex#324)) ([864fb52](webex@864fb52)) * taskList and IncomingTask widgets added ([webex#348](webex#348)) ([ce3a619](webex@ce3a619)) * **user-state:** load and change state, client timer ([webex#347](webex#347)) ([f1ccaeb](webex@f1ccaeb)) * **widget-cc-station-login:** Spark 575845 login widget ([webex#239](webex#239)) ([66b8a20](webex@66b8a20)) * **widgets:** added-relogin-logic ([webex#357](webex#357)) ([94dd415](webex@94dd415)) * **widgets:** shifted-timer-to-worker ([webex#352](webex#352)) ([c06fe9c](webex@c06fe9c))
JIRA - SPARK-601658
cc-taskpackagemobx-react-liteinstead ofmobx-reactas we are only using functional componentsSummary by CodeRabbit
Dependency Updates
mobx-reactwithmobx-react-litein multiple packagesBuild Process Improvements
clean:diststep before webpack builds across multiple packagesTesting Configuration
Webpack Configuration