-
Notifications
You must be signed in to change notification settings - Fork 514
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
Update license Component for Github SBOM #9755
base: develop
Are you sure you want to change the base?
Update license Component for Github SBOM #9755
Conversation
WalkthroughThe changes to the Changes
Assessment against linked issues
Poem
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. |
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: 0
🧹 Nitpick comments (7)
src/components/Licenses/SBOMViewer.tsx (7)
52-66
: Consider more robust error handling.
Currently, any fetch failure logs an error to the console and returnsnull
. Think about handling errors more gracefully in the UI, for instance by showing a user-facing alert or fallback message.
86-86
: Tab naming could be more descriptive.
Using "bom" vs "beBom" might be confusing. Renaming them to “frontendBom” / “backendBom” can improve clarity.
118-118
: Consider displaying additional SBOM metadata.
AlongsidespdxVersion
, you might also display thedataLicense
or other relevant metadata for a more comprehensive SBOM overview.
131-131
: Use stable keys when rendering lists.
Consider using a unique identifier from each package (e.g.,pkg.SPDXID
orpkg.name
) instead of the arrayindex
for improved React performance.
137-137
: Commented-out externalRefs link.
This might be leftover debug code or a temporary workaround. Consider removing or adding a rationale comment if intentionally disabled.
150-150
: Fallback anchor for unrecognized license IDs.
Navigating to "#" might not provide user feedback. Consider a placeholder page or tooltip to clarify no recognized license URL is available.
172-172
: Consider stable keys for external references.
Usingidx
as the key can lead to React diffing issues if references change order. Prefer a unique property from each reference, if available.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Licenses/SBOMViewer.tsx
(3 hunks)
🔇 Additional comments (14)
src/components/Licenses/SBOMViewer.tsx (14)
2-2
: Imports look good.
The newly added import for React hooks is appropriate and aligns with best practices in functional components.
15-27
: Validate optional fields for GitHubPackage.
All fields in this interface appear to match the GitHub SBOM format. Consider verifying whether any additional fields from GitHub’s SBOM API may need to be included or marked optional for better type safety.
30-41
: Interface structure aligns with GitHub SBOM data.
It’s good that each property is optional, preventing runtime errors when the SBOM might be missing certain fields.
49-51
: Separate state variables for frontend and backend SBOM.
Assigning each SBOM to its own state variable is clear and logical.
68-85
: Concise async loading for both FE/BE.
Fetching FE and BE SBOM data in parallel is efficient. If partial fetch success is acceptable (only one resource might load), confirm thatnull
states are handled correctly in the UI.
93-93
: Array fallback for safe rendering.
The usage of|| []
ensures no errors whenbomData
orsbom
isnull
. Nice defensive coding.
122-123
: Date formatting is handled gracefully.
Falling back to "N/A" if a date is missing ensures the UI won’t break.
129-129
: Package header is clear.
Labeling this section is helpful in guiding the user to the package list.
143-143
: Package name and version.
Displaying both name and version in the same heading is user-friendly and concise.
146-146
: Conditional rendering of the license.
Filtering out packages without alicenseConcluded
field reduces UI clutter. Implementation is clear.
155-155
: License fallback "N/A."
Good to maintain a sensible default text if the license field is somehow empty.
175-175
: External reference link fallback.
IfreferenceLocator
is missing, a fallback of "#" prevents errors. This maintains a functioning UI.
178-178
: Optional reference locator text.
Gracefully handles missing locator content by providing "N/A."
180-182
: Category display is optional.
The checks forref.referenceCategory
andref.referenceLocator
align with the optional nature of external references.
useEffect(() => { | ||
const fetchSBOMData = async (url: string): Promise<GitHubSBOM | null> => { | ||
try { | ||
const response = await fetch(url, { | ||
headers: { | ||
Accept: "application/vnd.github+json", | ||
"X-GitHub-Api-Version": "2022-11-28", | ||
}, | ||
}); | ||
return response.ok ? await response.json() : null; | ||
} catch (error) { | ||
console.error("Error fetching SBOM data:", error); | ||
return null; | ||
} | ||
}; | ||
|
||
const fetchData = async () => { | ||
const feUrl = | ||
"https://api.github.com/repos/ohcnetwork/care_fe/dependency-graph/sbom"; | ||
const beUrl = | ||
"https://api.github.com/repos/ohcnetwork/care/dependency-graph/sbom"; | ||
|
||
const [frontendData, backendData] = await Promise.all([ | ||
fetchSBOMData(feUrl), | ||
fetchSBOMData(beUrl), | ||
]); | ||
|
||
setFeBomData(frontendData); | ||
setBeBomData(backendData); | ||
}; | ||
|
||
fetchData(); | ||
}, []); |
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.
You can use useQuery with fetch instead.
Also, instead of fetching dynamically everytime on runtime, i'd recommend adding this data-fetching logic to postinstall
step or build step.
Refer npm scripts: https://docs.npmjs.com/cli/v9/using-npm/scripts#examples
During the step, fetch the data using gh's APIs and save it as a JSON (gitignore it).
Lazy import the JSON directly in this file, and render accordingly.
cc: @bodhish
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: 3
🧹 Nitpick comments (2)
scripts/fetchSbomData.js (1)
49-49
: Add proper error handling for unhandled rejections.The script should handle unhandled promise rejections properly.
Add this error handling:
+process.on('unhandledRejection', (error) => { + console.error('Unhandled promise rejection:', error); + process.exit(1); +}); + fetchData();src/components/Licenses/SBOMViewer.tsx (1)
Line range hint
134-157
: Add error handling for clipboard operations.The clipboard functionality should handle potential errors.
- const handleCopy = () => { + const handleCopy = (text: string, result: boolean) => { + if (!result) { + console.error('Failed to copy to clipboard'); + return; + } setCopyStatus(true); setTimeout(() => setCopyStatus(false), 2000); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.gitignore
(1 hunks)package.json
(1 hunks)scripts/fetchSbomData.js
(1 hunks)src/components/Licenses/SBOMViewer.tsx
(4 hunks)
🧰 Additional context used
🪛 GitHub Actions: Lint Code Base
package.json
[warning] Node.js version incompatibility - Required: >=22.8.0, Current: v20.18.1
[warning] Multiple deprecated package dependencies: sourcemap-codec, rimraf, npmlog, inflight, gauge, are-we-there-yet, boolean, @humanwhocodes/config-array, @humanwhocodes/object-schema, glob, eslint
scripts/fetchSbomData.js
[error] 1-1: Cannot use import statement outside a module. Need to set "type": "module" in package.json or use .mjs extension
🔇 Additional comments (5)
.gitignore (1)
68-70
: LGTM! Appropriate entries added to .gitignore.The new entries correctly exclude the generated SBOM data files from version control.
src/components/Licenses/SBOMViewer.tsx (2)
1-13
: LGTM! Good implementation of i18n and SBOM data imports.The changes properly implement internationalization and correctly update the SBOM data imports to use the new GitHub SBOM format.
Line range hint
34-133
: LGTM! Well-structured UI with proper i18n implementation.The UI components are well-organized with proper internationalization of all strings and good TypeScript/React patterns.
package.json (2)
38-38
: LGTM! Appropriate addition to postinstall script.The script is correctly added to run after platform dependencies are installed.
🧰 Tools
🪛 GitHub Actions: Lint Code Base
[warning] Node.js version incompatibility - Required: >=22.8.0, Current: v20.18.1
[warning] Multiple deprecated package dependencies: sourcemap-codec, rimraf, npmlog, inflight, gauge, are-we-there-yet, boolean, @humanwhocodes/config-array, @humanwhocodes/object-schema, glob, eslint
Line range hint
1-1
: Update Node.js version and deprecated dependencies.The pipeline shows several issues that need attention:
- Node.js version incompatibility (required >=22.8.0, current v20.18.1)
- Multiple deprecated package dependencies
Please update the Node.js version in your CI environment and update the following deprecated packages:
- sourcemap-codec
- rimraf
- npmlog
- inflight
- gauge
- are-we-there-yet
- boolean
- @humanwhocodes/config-array
- @humanwhocodes/object-schema
- glob
- eslint
Run this script to check for the latest versions:
🧰 Tools
🪛 GitHub Actions: Lint Code Base
[warning] Node.js version incompatibility - Required: >=22.8.0, Current: v20.18.1
[warning] Multiple deprecated package dependencies: sourcemap-codec, rimraf, npmlog, inflight, gauge, are-we-there-yet, boolean, @humanwhocodes/config-array, @humanwhocodes/object-schema, glob, eslint
import fs from "fs"; | ||
import fetch from "node-fetch"; | ||
|
||
const fetchSBOMData = async (url) => { | ||
const response = await fetch(url, { | ||
headers: { | ||
Accept: "application/vnd.github+json", | ||
"X-GitHub-Api-Version": "2022-11-28", | ||
}, | ||
}); | ||
if (!response.ok) { | ||
throw new Error(`Error fetching SBOM data from ${url}`); | ||
} | ||
return await response.json(); | ||
}; |
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.
Fix ESM compatibility and add GitHub authentication.
The code has several issues that need to be addressed:
- ES modules syntax requires "type": "module" in package.json
- GitHub API calls should use authentication to avoid rate limiting
- API version should be configurable for future compatibility
Apply these changes:
+// Add to package.json
+{
+ "type": "module"
+}
import fs from "fs";
import fetch from "node-fetch";
+import dotenv from "dotenv";
+
+dotenv.config();
+
+const GITHUB_TOKEN = process.env.GITHUB_TOKEN;
const fetchSBOMData = async (url) => {
const response = await fetch(url, {
headers: {
Accept: "application/vnd.github+json",
+ Authorization: `Bearer ${GITHUB_TOKEN}`,
"X-GitHub-Api-Version": "2022-11-28",
},
});
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: Lint Code Base
[error] 1-1: Cannot use import statement outside a module. Need to set "type": "module" in package.json or use .mjs extension
const fetchData = async () => { | ||
const feUrl = | ||
"https://api.github.com/repos/ohcnetwork/care_fe/dependency-graph/sbom"; | ||
const beUrl = | ||
"https://api.github.com/repos/ohcnetwork/care/dependency-graph/sbom"; | ||
|
||
try { | ||
const [frontendData, backendData] = await Promise.all([ | ||
fetchSBOMData(feUrl), | ||
fetchSBOMData(beUrl), | ||
]); | ||
|
||
// Write frontend SBOM data | ||
fs.writeFileSync( | ||
"./src/components/Licenses/feBomData.json", | ||
JSON.stringify(frontendData, null, 2), | ||
); | ||
|
||
// Write backend SBOM data | ||
fs.writeFileSync( | ||
"./src/components/Licenses/beBomData.json", | ||
JSON.stringify(backendData, null, 2), | ||
); | ||
|
||
console.log( | ||
"SBOM data successfully saved as feBomData.json and beBomData.json", | ||
); | ||
} catch (error) { | ||
console.error("Error fetching SBOM data:", error.message); | ||
} | ||
}; |
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.
🛠️ Refactor suggestion
Improve error handling and configuration management.
The function needs several improvements:
- Repository URLs should be configurable
- File operations need proper error handling
- SBOM data structure should be validated
Apply these changes:
+const CONFIG = {
+ repositories: {
+ frontend: "ohcnetwork/care_fe",
+ backend: "ohcnetwork/care"
+ },
+ outputPath: "./src/components/Licenses"
+};
+
+const validateSBOMData = (data) => {
+ if (!data?.sbom?.packages) {
+ throw new Error("Invalid SBOM data structure");
+ }
+};
+
const fetchData = async () => {
- const feUrl =
- "https://api.github.com/repos/ohcnetwork/care_fe/dependency-graph/sbom";
- const beUrl =
- "https://api.github.com/repos/ohcnetwork/care/dependency-graph/sbom";
+ const feUrl = `https://api.github.com/repos/${CONFIG.repositories.frontend}/dependency-graph/sbom`;
+ const beUrl = `https://api.github.com/repos/${CONFIG.repositories.backend}/dependency-graph/sbom`;
try {
const [frontendData, backendData] = await Promise.all([
fetchSBOMData(feUrl),
fetchSBOMData(beUrl),
]);
+ validateSBOMData(frontendData);
+ validateSBOMData(backendData);
+
+ fs.mkdirSync(CONFIG.outputPath, { recursive: true });
+
// Write frontend SBOM data
fs.writeFileSync(
- "./src/components/Licenses/feBomData.json",
+ `${CONFIG.outputPath}/feBomData.json`,
JSON.stringify(frontendData, null, 2),
);
// Write backend SBOM data
fs.writeFileSync(
- "./src/components/Licenses/beBomData.json",
+ `${CONFIG.outputPath}/beBomData.json`,
JSON.stringify(backendData, null, 2),
);
console.log(
"SBOM data successfully saved as feBomData.json and beBomData.json",
);
} catch (error) {
console.error("Error fetching SBOM data:", error.message);
+ process.exit(1);
}
};
📝 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.
const fetchData = async () => { | |
const feUrl = | |
"https://api.github.com/repos/ohcnetwork/care_fe/dependency-graph/sbom"; | |
const beUrl = | |
"https://api.github.com/repos/ohcnetwork/care/dependency-graph/sbom"; | |
try { | |
const [frontendData, backendData] = await Promise.all([ | |
fetchSBOMData(feUrl), | |
fetchSBOMData(beUrl), | |
]); | |
// Write frontend SBOM data | |
fs.writeFileSync( | |
"./src/components/Licenses/feBomData.json", | |
JSON.stringify(frontendData, null, 2), | |
); | |
// Write backend SBOM data | |
fs.writeFileSync( | |
"./src/components/Licenses/beBomData.json", | |
JSON.stringify(backendData, null, 2), | |
); | |
console.log( | |
"SBOM data successfully saved as feBomData.json and beBomData.json", | |
); | |
} catch (error) { | |
console.error("Error fetching SBOM data:", error.message); | |
} | |
}; | |
const CONFIG = { | |
repositories: { | |
frontend: "ohcnetwork/care_fe", | |
backend: "ohcnetwork/care" | |
}, | |
outputPath: "./src/components/Licenses" | |
}; | |
const validateSBOMData = (data) => { | |
if (!data?.sbom?.packages) { | |
throw new Error("Invalid SBOM data structure"); | |
} | |
}; | |
const fetchData = async () => { | |
const feUrl = `https://api.github.com/repos/${CONFIG.repositories.frontend}/dependency-graph/sbom`; | |
const beUrl = `https://api.github.com/repos/${CONFIG.repositories.backend}/dependency-graph/sbom`; | |
try { | |
const [frontendData, backendData] = await Promise.all([ | |
fetchSBOMData(feUrl), | |
fetchSBOMData(beUrl), | |
]); | |
validateSBOMData(frontendData); | |
validateSBOMData(backendData); | |
fs.mkdirSync(CONFIG.outputPath, { recursive: true }); | |
// Write frontend SBOM data | |
fs.writeFileSync( | |
`${CONFIG.outputPath}/feBomData.json`, | |
JSON.stringify(frontendData, null, 2), | |
); | |
// Write backend SBOM data | |
fs.writeFileSync( | |
`${CONFIG.outputPath}/beBomData.json`, | |
JSON.stringify(backendData, null, 2), | |
); | |
console.log( | |
"SBOM data successfully saved as feBomData.json and beBomData.json", | |
); | |
} catch (error) { | |
console.error("Error fetching SBOM data:", error.message); | |
process.exit(1); | |
} | |
}; |
const bomData = activeTab === "bom" ? feBomData : beBomData; | ||
|
||
const handleCopy = () => { | ||
setCopyStatus(true); | ||
setTimeout(() => setCopyStatus(false), 2000); | ||
}; | ||
|
||
const bomData = (activeTab === "bom" ? feBomData : beBomData) as CycloneDXBOM; | ||
const packages = bomData?.sbom?.packages || []; |
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.
🛠️ Refactor suggestion
Add TypeScript interfaces for the GitHub SBOM format.
The code lacks type definitions for the new GitHub SBOM format, which could lead to runtime errors.
Add these type definitions:
interface GitHubSBOM {
sbom: {
spdxVersion: string;
creationInfo: {
created: string;
};
packages: Array<{
name: string;
versionInfo?: string;
licenseConcluded?: string;
externalRefs?: Array<{
referenceLocator?: string;
referenceCategory?: string;
}>;
}>;
};
}
const bomData = (activeTab === "bom" ? feBomData : beBomData) as GitHubSBOM;
const packages = bomData?.sbom?.packages || [];
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Refactor