Update Care Apps Setup; API URL on window #9883
Conversation
WalkthroughThis pull request introduces changes to environment configuration and application setup across multiple files. The modifications focus on enhancing the handling of remote application configurations, particularly for the Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
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 (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Deploying care-fe with
|
| Latest commit: |
598433e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c9ac5317.care-fe.pages.dev |
| Branch Preview URL: | https://care-apps-setup.care-fe.pages.dev |
![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
🧹 Nitpick comments (5)
vite.config.mts (1)
68-71: Consider extractingParsedRemoteConfigto a shared moduleThe
ParsedRemoteConfiginterface is defined both here and inscripts/setup-care-apps.ts. To promote reusability and maintain consistency, consider moving this interface to a shared module that can be imported where needed.src/index.tsx (1)
10-14: Avoid augmenting the globalWindowinterfaceAugmenting the global
Windowinterface by addingCARE_API_URLmay lead to potential conflicts and make the code harder to maintain. Consider using a configuration module or context provider to manage and distribute environment variables within your application.scripts/setup-care-apps.ts (2)
19-23: ExtractParsedRemoteConfigto a shared moduleThe
ParsedRemoteConfiginterface is also defined invite.config.mts. To maintain consistency and reduce duplication, consider moving this interface to a shared module that both scripts can import.
73-77: Use a reliable utility for converting repo names to camelCaseThe current method for converting repository names to camelCase may not handle all edge cases, especially with complex names. Consider using a well-tested utility like
lodash.camelCaseto ensure consistent and accurate conversion.Apply this diff to implement the utility:
+import camelCase from "lodash.camelcase"; ... const plugins = process.env.REACT_ENABLED_APPS.split(",").map((app) => { const { repo } = parseRemoteConfig(app); return { repo, - // Convert repo name to camelCase for import - camelCaseName: repo - .replace(/[-_]/g, "") - .replace(/\b\w/g, (char, index) => - index === 0 ? char.toLowerCase() : char.toUpperCase(), - ), + camelCaseName: camelCase(repo), }; });Ensure that you have
lodash.camelcaseinstalled in your dependencies..env (1)
17-17: Consider adding development setup documentation.The change to use localhost:4173 requires additional setup context for developers.
Consider:
- Document port requirements in README.md
- Add validation for port availability during setup
- Include fallback configuration for CI/CD environments
- Document the new format specification for remote apps configuration
Would you like me to help draft this documentation?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.env(1 hunks)scripts/setup-care-apps.ts(1 hunks)src/index.tsx(1 hunks)vite.config.mts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (3)
src/index.tsx (1)
17-17: Ensure safe exposure of environment variablesAssigning
import.meta.env.REACT_CARE_API_URLdirectly towindow.CARE_API_URLmay expose sensitive information to the client-side, which could be a security concern. Verify thatREACT_CARE_API_URLdoes not contain any sensitive data and that exposing it complies with security policies..env (2)
17-17: Validate development environment setup.The use of a fixed port (4173) for local development could lead to port conflicts.
Let's check for potential port conflicts in the codebase:
17-17: Verify the new remote app configuration format.The format change from URL-based to host-based configuration (
@localhost:4173) appears to be a breaking change. This could impact other environments and developers.Let's verify the usage and documentation of this new format:
| function parseRemoteConfig(appConfig: string): ParsedRemoteConfig { | ||
| // Handle custom URLs (both localhost and custom hosted) | ||
| if (appConfig.includes("@")) { | ||
| const [package_, url] = appConfig.split("@"); | ||
| const [org, repo] = package_.split("/"); | ||
|
|
||
| // Check if the module is an entry point | ||
| if (modInfo.isEntry) { | ||
| return true; | ||
| } | ||
| // Add appropriate protocol based on whether it's localhost | ||
| const protocol = url.includes("localhost") ? "http://" : "https://"; | ||
| const fullUrl = url.startsWith("http") ? url : `${protocol}${url}`; | ||
|
|
||
| // Check all static importers | ||
| for (const importerId of modInfo.importers) { | ||
| if (isStaticallyImportedByEntry(getModuleInfo, importerId, visited)) { | ||
| return true; | ||
| } | ||
| return { | ||
| url: `${fullUrl}/assets/remoteEntry.js`, | ||
| org, | ||
| repo, | ||
| }; | ||
| } | ||
|
|
||
| return false; | ||
| // Handle GitHub Pages URLs | ||
| const [org, repo] = appConfig.split("/"); | ||
| return { | ||
| url: `https://${org}.github.io/${repo}/assets/remoteEntry.js`, | ||
| org, | ||
| repo, | ||
| }; |
There was a problem hiding this comment.
Add error handling for invalid appConfig formats in parseRemoteConfig
The parseRemoteConfig function assumes that appConfig is always in the correct format ("org/repo" or "org/repo@url"). If appConfig is malformed or missing expected delimiters, the destructuring assignments may result in undefined values for org, repo, or url, leading to runtime errors.
Apply this diff to add validation checks:
function parseRemoteConfig(appConfig: string): ParsedRemoteConfig {
+ if (!appConfig.includes("/")) {
+ throw new Error(`Invalid app configuration format: ${appConfig}. Expected 'org/repo' or 'org/repo@url'.`);
+ }
// Handle custom URLs (both localhost and custom hosted)
if (appConfig.includes("@")) {
const [package_, url] = appConfig.split("@");
const [org, repo] = package_.split("/");
+ if (!org || !repo || !url) {
+ throw new Error(`Invalid custom URL configuration: ${appConfig}. Expected 'org/repo@url'.`);
+ }
// Add appropriate protocol based on whether it's localhost
const protocol = url.includes("localhost") ? "http://" : "https://";
const fullUrl = url.startsWith("http") ? url : `${protocol}${url}`;
return {
url: `${fullUrl}/assets/remoteEntry.js`,
org,
repo,
};
}
// Handle GitHub Pages URLs
const [org, repo] = appConfig.split("/");
+ if (!org || !repo) {
+ throw new Error(`Invalid GitHub Pages configuration: ${appConfig}. Expected 'org/repo'.`);
+ }
return {
url: `https://${org}.github.io/${repo}/assets/remoteEntry.js`,
org,
repo,
};
}📝 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.
| function parseRemoteConfig(appConfig: string): ParsedRemoteConfig { | |
| // Handle custom URLs (both localhost and custom hosted) | |
| if (appConfig.includes("@")) { | |
| const [package_, url] = appConfig.split("@"); | |
| const [org, repo] = package_.split("/"); | |
| // Check if the module is an entry point | |
| if (modInfo.isEntry) { | |
| return true; | |
| } | |
| // Add appropriate protocol based on whether it's localhost | |
| const protocol = url.includes("localhost") ? "http://" : "https://"; | |
| const fullUrl = url.startsWith("http") ? url : `${protocol}${url}`; | |
| // Check all static importers | |
| for (const importerId of modInfo.importers) { | |
| if (isStaticallyImportedByEntry(getModuleInfo, importerId, visited)) { | |
| return true; | |
| } | |
| return { | |
| url: `${fullUrl}/assets/remoteEntry.js`, | |
| org, | |
| repo, | |
| }; | |
| } | |
| return false; | |
| // Handle GitHub Pages URLs | |
| const [org, repo] = appConfig.split("/"); | |
| return { | |
| url: `https://${org}.github.io/${repo}/assets/remoteEntry.js`, | |
| org, | |
| repo, | |
| }; | |
| function parseRemoteConfig(appConfig: string): ParsedRemoteConfig { | |
| if (!appConfig.includes("/")) { | |
| throw new Error(`Invalid app configuration format: ${appConfig}. Expected 'org/repo' or 'org/repo@url'.`); | |
| } | |
| // Handle custom URLs (both localhost and custom hosted) | |
| if (appConfig.includes("@")) { | |
| const [package_, url] = appConfig.split("@"); | |
| const [org, repo] = package_.split("/"); | |
| if (!org || !repo || !url) { | |
| throw new Error(`Invalid custom URL configuration: ${appConfig}. Expected 'org/repo@url'.`); | |
| } | |
| // Add appropriate protocol based on whether it's localhost | |
| const protocol = url.includes("localhost") ? "http://" : "https://"; | |
| const fullUrl = url.startsWith("http") ? url : `${protocol}${url}`; | |
| return { | |
| url: `${fullUrl}/assets/remoteEntry.js`, | |
| org, | |
| repo, | |
| }; | |
| } | |
| // Handle GitHub Pages URLs | |
| const [org, repo] = appConfig.split("/"); | |
| if (!org || !repo) { | |
| throw new Error(`Invalid GitHub Pages configuration: ${appConfig}. Expected 'org/repo'.`); | |
| } | |
| return { | |
| url: `https://${org}.github.io/${repo}/assets/remoteEntry.js`, | |
| org, | |
| repo, | |
| }; | |
| } |
| function parseRemoteConfig(appConfig: string): ParsedRemoteConfig { | ||
| // Handle custom URLs (both localhost and custom hosted) | ||
| if (appConfig.includes("@")) { | ||
| const [package_] = appConfig.split("@"); | ||
| const [org, repo] = package_.split("/"); | ||
| return { | ||
| url: "", // URL not needed for plugin setup | ||
| org, | ||
| repo, | ||
| }; | ||
| } | ||
|
|
||
| // Handle GitHub Pages URLs | ||
| const [org, repo] = appConfig.split("/"); | ||
| return { | ||
| url: "", // URL not needed for plugin setup | ||
| org, | ||
| repo, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Add error handling for malformed appConfig in parseRemoteConfig
The parseRemoteConfig function assumes a specific format for appConfig. If the input is malformed, destructuring may result in undefined values for org and repo. This can cause unexpected behavior or runtime errors.
Apply this diff to include validation:
function parseRemoteConfig(appConfig: string): ParsedRemoteConfig {
+ if (!appConfig.includes("/")) {
+ throw new Error(`Invalid app configuration format: ${appConfig}. Expected 'org/repo' or 'org/repo@url'.`);
+ }
// Handle custom URLs (both localhost and custom hosted)
if (appConfig.includes("@")) {
const [package_] = appConfig.split("@");
const [org, repo] = package_.split("/");
+ if (!org || !repo) {
+ throw new Error(`Invalid custom URL configuration: ${appConfig}. Expected 'org/repo@url'.`);
+ }
return {
url: "", // URL not needed for plugin setup
org,
repo,
};
}
// Handle GitHub Pages URLs
const [org, repo] = appConfig.split("/");
+ if (!org || !repo) {
+ throw new Error(`Invalid GitHub Pages configuration: ${appConfig}. Expected 'org/repo'.`);
+ }
return {
url: "", // URL not needed for plugin setup
org,
repo,
};
}📝 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.
| function parseRemoteConfig(appConfig: string): ParsedRemoteConfig { | |
| // Handle custom URLs (both localhost and custom hosted) | |
| if (appConfig.includes("@")) { | |
| const [package_] = appConfig.split("@"); | |
| const [org, repo] = package_.split("/"); | |
| return { | |
| url: "", // URL not needed for plugin setup | |
| org, | |
| repo, | |
| }; | |
| } | |
| // Handle GitHub Pages URLs | |
| const [org, repo] = appConfig.split("/"); | |
| return { | |
| url: "", // URL not needed for plugin setup | |
| org, | |
| repo, | |
| }; | |
| } | |
| function parseRemoteConfig(appConfig: string): ParsedRemoteConfig { | |
| if (!appConfig.includes("/")) { | |
| throw new Error(`Invalid app configuration format: ${appConfig}. Expected 'org/repo' or 'org/repo@url'.`); | |
| } | |
| // Handle custom URLs (both localhost and custom hosted) | |
| if (appConfig.includes("@")) { | |
| const [package_] = appConfig.split("@"); | |
| const [org, repo] = package_.split("/"); | |
| if (!org || !repo) { | |
| throw new Error(`Invalid custom URL configuration: ${appConfig}. Expected 'org/repo@url'.`); | |
| } | |
| return { | |
| url: "", // URL not needed for plugin setup | |
| org, | |
| repo, | |
| }; | |
| } | |
| // Handle GitHub Pages URLs | |
| const [org, repo] = appConfig.split("/"); | |
| if (!org || !repo) { | |
| throw new Error(`Invalid GitHub Pages configuration: ${appConfig}. Expected 'org/repo'.`); | |
| } | |
| return { | |
| url: "", // URL not needed for plugin setup | |
| org, | |
| repo, | |
| }; | |
| } |
| @@ -14,4 +14,4 @@ ESLINT_NO_DEV_ERRORS=true | |||
| CARE_CDN_URL="https://egov-s3-facility-10bedicu.s3.amazonaws.com https://egov-s3-patient-data-10bedicu.s3.amazonaws.com http://localhost:4566" | |||
| REACT_ALLOWED_LOCALES="en,hi,ta,ml,mr,kn" | |||
|
|
|||
| REACT_ENABLED_APPS="https://care-scribe-fe.pages.dev|ohcnetwork/care_scribe_fe" | |||
| REACT_ENABLED_APPS="ohcnetwork/care_scribe_fe@localhost:4173" | |||
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Configuration Updates
REACT_ENABLED_APPSto use local development setupDevelopment Improvements