-
Notifications
You must be signed in to change notification settings - Fork 54
fix(bundling): fixed the multiple React instance and useRef errors #355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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 (
|
packages/contact-center/station-login/src/station-login/index.tsx
Outdated
Show resolved
Hide resolved
@@ -1,6 +1,6 @@ | |||
import React from 'react'; | |||
import store from '@webex/cc-store'; | |||
import {observer} from 'mobx-react'; | |||
import {observer} from 'mobx-react-lite'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed back to mobx-react
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rarajes2 @mkesavan13 @Shreyas281299 would it be better to use mobx-react-lite since it's a smaller module than mobx-react? We should try to conserve size wherever possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am in favour of using mobx-react-lite
. Let's hear from @mkesavan13 and @Shreyas281299 then we can decide
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of nitpicks. Looks good otherwise
"build": "yarn run -T tsc", | ||
"build:src": "webpack && yarn run build", | ||
"build:src": "webpack", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we consider calling clean in individual widgets instead of the global package.json
command?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are at individual package level, we run all of them once with workspaces foreach ...
command. Correct me if I misunderstood
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in the PR - #356
console.log('StationLogin: Teams >>', teams); | ||
console.log('StationLogin: Login Options >>', loginOptions); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need these console statements anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in the PR - #356
🎉 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))
externals
parameter in thewebpack.config.js
for all the widget packages that use React.react
andreact-dom
topeer-dependencies
for our widget packages so that we pass responsibility to samples app to includereact
@webex/cc-store
asexternals
parameter in the webpack config as it is a shared package and required to be shared as only one copy of instance.Additional things to test:
alias
from webpack config -> Tested this and removed. It's not requiredresolutions
from package.json of react-samples -> Tested this and removed. It's not required.