-
Notifications
You must be signed in to change notification settings - Fork 6
Libraries changes and cleanup (#245) #246
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
base: develop-webpack
Are you sure you want to change the base?
Conversation
* SVG Components cleanup * ui-components cleanup * react-components changes * libraries cleanup and changes
WalkthroughThis update revises several library modules. The Changes
Sequence Diagram(s)sequenceDiagram
participant SS as StoreService (digitInitData)
participant LS as LocalizationService
participant C as Console
SS->>LS: getLocale()
alt Successful retrieval
LS-->>SS: locale data
else Error encountered
LS-->>SS: throw error
SS->>C: log(error)
end
sequenceDiagram
participant I as initI18n
participant i18n as i18next
participant CB as Callback
I->>i18n: initialize i18next
i18n-->>I: initialization complete
alt If callback exists
I->>CB: invoke callback
end
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
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.
@purepiyush can u check them
@@ -89,7 +89,7 @@ export const StoreService = { | |||
uiHomePage: uiHomePage, | |||
}; | |||
}, | |||
digitInitData: async (stateCode, enabledModules, modulePrefix) => { | |||
digitInitData: async (stateCode="mz", enabledModules, modulePrefix) => { |
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.
we cant have hardcoded @purepiyush
@@ -1471,7 +1471,7 @@ const debouncedCall = ({ serviceName, url, data, useCache, params }, resolve, re | |||
}; | |||
|
|||
export const MdmsService = { | |||
init: (stateCode) => | |||
init: (stateCode="mz") => |
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.
same here as well
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.
Actionable comments posted: 1
🧹 Nitpick comments (4)
webpack/libraries/src/services/elements/Localization/service.js (1)
62-62
: Consistent use of global i18next instanceExplicitly referencing
window.i18next
ensures that the globally initialized instance is used, which aligns with how it's set up intranslations/index.js
(line 58) and prevents potential conflicts with multiple i18next instances.Note that line 88 in this same file still uses
i18next.changeLanguage(locale)
without thewindow.
prefix. For consistency, consider updating that line too.- i18next.changeLanguage(locale); + window.i18next.changeLanguage(locale);webpack/libraries/src/hooks/store.js (1)
7-13
: Updated useQuery implementation to use object parameter styleThe change from positional arguments to an object parameter style aligns with modern React Query API patterns, making the code more maintainable and self-documenting.
However, the commented-out empty object and inconsistent spacing in the code are unnecessary and could be cleaned up:
return useQuery({ queryKey : ['store', stateCode, moduleCode, language, modulePrefix], // queryKey queryFn : () => StoreService.defaultData(stateCode, moduleCode, language, modulePrefix), // query function - // { - // // You can add other options here if needed, such as `enabled`, `staleTime`, etc. - // } }webpack/libraries/src/index.js (2)
7-9
: Unnecessary comment separatorsThe comment separators (
// ---
) don't add value and aren't standard practice. Consider removing them for cleaner code.-// --- import { StoreService } from "./services/molecules/Store/service"; -// ---
88-90
: Empty comments and StoreService integrationThe empty comments (
//
) don't provide any value and should be removed. Additionally, ensure that the newly integrated StoreService is properly documented for other developers.- // setupLibraries("StoreServiceLib", StoreService); - //
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
webpack/libraries/package.json
is excluded by!**/*.json
📒 Files selected for processing (6)
webpack/libraries/src/hooks/store.js
(1 hunks)webpack/libraries/src/index.js
(2 hunks)webpack/libraries/src/services/elements/Localization/service.js
(1 hunks)webpack/libraries/src/services/elements/MDMS.js
(2 hunks)webpack/libraries/src/services/molecules/Store/service.js
(3 hunks)webpack/libraries/src/translations/index.js
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
webpack/libraries/src/hooks/store.js (1)
webpack/libraries/src/services/molecules/Store/service.js (2)
StoreService
(40-217)StoreService
(40-217)
webpack/libraries/src/services/elements/MDMS.js (7)
webpack/libraries/src/hooks/pgr/useComplaintStatusWithCount.js (1)
tenantId
(7-7)webpack/libraries/src/hooks/pgr/useComplaintStatus.js (1)
tenantId
(7-7)webpack/libraries/src/services/elements/Complaint.js (1)
tenantId
(18-18)webpack/libraries/src/services/elements/DSS.js (1)
tenantId
(26-26)webpack/libraries/src/services/molecules/Ulb/index.js (2)
tenantId
(42-42)tenantId
(79-79)webpack/libraries/src/services/elements/WorkFlow.js (1)
tenantId
(22-22)webpack/libraries/src/utils/index.js (1)
tenantId
(195-195)
webpack/libraries/src/services/elements/Localization/service.js (2)
webpack/libraries/src/hooks/pgr/useComplaintDetails.js (1)
window
(31-31)webpack/libraries/src/utils/pdf.js (2)
locale
(160-160)locale
(239-239)
🔇 Additional comments (6)
webpack/libraries/src/translations/index.js (1)
59-59
: Good defensive programming practice addedAdding a conditional check before invoking the callback improves function robustness by preventing potential runtime errors when the callback isn't provided.
webpack/libraries/src/index.js (1)
97-97
: Verify exports are consistent with changesThe
Hooks
module is still being exported even though its setup has been commented out. This could be misleading for consumers of this library. Consider removingHooks
from the exports or documenting that it's being phased out.-export { initLibraries, Enums, Hooks, subFormRegistry ,Request}; +export { initLibraries, Enums, subFormRegistry, Request, StoreService };webpack/libraries/src/services/molecules/Store/service.js (2)
92-92
: Good addition of default state code.Adding a default value for
stateCode
parameter improves robustness by ensuring the function works even when called without this parameter. The "mz" default is consistent with similar changes in other files.
180-188
: Excellent error handling improvement.Adding try-catch error handling around the LocalizationService call prevents the application from crashing if localization data fetching fails. This makes the initialization process more robust and improves user experience by allowing the application to continue running even if translations can't be loaded.
webpack/libraries/src/services/elements/MDMS.js (2)
67-67
: Good addition of default tenant ID.Adding a default value for the
tenantId
parameter improves code reliability and reduces potential errors when this function is called without the parameter. The "mz" default is consistent with other changes in the codebase.
1474-1474
: Good addition of default state code to init method.Adding a default value for the
stateCode
parameter in the MDMS init method is consistent with the other changes and provides a fallback value when the parameter is not provided. This maintains consistency throughout the codebase.
@@ -76,12 +79,15 @@ const initLibraries = () => { | |||
setupLibraries("InboxGeneral", InboxGeneral); | |||
setupLibraries("ShareFiles", ShareFiles); | |||
setupLibraries("Contexts", Contexts); | |||
setupLibraries("Hooks", Hooks); | |||
// setupLibraries("Hooks", Hooks); |
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.
💡 Verification agent
🧩 Analysis chain
Hooks setup disabled
The Hooks setup has been commented out, which seems to be part of the architectural change moving to StoreService. Ensure that all functionality previously handled by Hooks is now properly handled by StoreService or document this change as a potential breaking change.
Recommended checks:
- Verify if any existing components are still trying to use functionality from the Hooks module
- Confirm that StoreService covers all relevant functionality
🏁 Script executed:
#!/bin/bash
# Find components that might be importing or using Hooks directly
rg "import.*Hooks.*from" --type js --type jsx --type ts --type tsx
rg "Digit\.Hooks\." --type js --type jsx --type ts --type tsx
Length of output: 188
I'll generate new shell scripts that explicitly add file types for JSX/TSX so we can verify if any components still import or reference the Hooks module. Please run the following scripts and share the output to confirm if any references remain:
Once you run these commands, please verify whether any output is returned. If no references are found, it likely indicates that the Hooks module is indeed no longer in use. If there are references, you'll need to ensure that either those components are updated to use StoreService or the change is documented properly as a potential breaking change.
🏁 Script executed:
#!/bin/bash
# Adding custom file types for jsx and tsx before searching for Hooks references.
# Search for imports of Hooks (works with .js, .jsx, .ts, .tsx files)
rg --type-add 'jsx:*.jsx' --type-add 'tsx:*.tsx' -e "import.*Hooks.*from" .
# Search for explicit usage of Digit.Hooks (works with .js, .jsx, .ts, .tsx files)
rg --type-add 'jsx:*.jsx' --type-add 'tsx:*.tsx' -e "Digit\.Hooks\." .
Length of output: 25779
Action: Address Incomplete Hooks Migration
The Hooks setup in webpack/libraries/src/index.js
is now disabled (see the commented out code), but our repository search reveals that many components continue to import and actively use Digit.Hooks
(e.g., useCustomAPIHook
, useQueryParams
, useCustomMDMS
, etc.) across various modules. This indicates that the architectural migration to StoreService
may be incomplete.
Please ensure one of the following:
- Fully migrate Hooks functionality: Update all components using
Digit.Hooks
to useStoreService
where applicable. - Document the change as a breaking change: Clearly document which Hooks functionalities are deprecated and how their absence is handled or should be manually addressed by downstream consumers.
Key Points:
- The file
webpack/libraries/src/index.js
(Line 82) contains the disabled setup:// setupLibraries("Hooks", Hooks);
- Numerous components in both the React and Webpack directories still reference
Digit.Hooks.*
.
Please review all Hooks usage to confirm coverage by StoreService
and update or document accordingly.
@purepiyush remove this logs webpack/ui-components/src/hoc/FieldController.js |
SVG Components cleanup
ui-components cleanup
react-components changes
libraries cleanup and changes