Refresh deps #632
Refresh deps #632
Conversation
first round of non breaking updates
WalkthroughThe changes reflect a shift towards stricter type safety, enhanced script execution control in npm, a transition from npm to pnpm for package management, and adjustments in styling and configurations. There's also a move to update the map library expressions and streamline the deployment process with new scripts, indicating a focus on modernization and efficiency improvements. Changes
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? TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- package.json
- pnpm-lock.yaml
Files selected for processing (6)
- .npmrc (1 hunks)
- src/core/draw_tools/layers/LocalEditableGeojsonLayer.ts (2 hunks)
- src/core/logical_layers/renderers/stylesConfigs/mcda/types.ts (1 hunks)
- src/core/logical_layers/utils/logicalLayerFabric.ts (1 hunks)
- src/features/live_sensor/tests/FakeSensor.ts (2 hunks)
- src/features/mcda/parser/validator.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- .npmrc
Additional comments: 7
src/core/draw_tools/layers/LocalEditableGeojsonLayer.ts (2)
86-91: The removal of the conditional logic and direct assignment of an empty array to
data.featuressimplifies the code. However, ensure that this change does not affect the functionality wheredata.featuresis expected to be a transformed version ofthis.props.data.features.108-109: The restructuring of the destructuring assignment for
this.props.geojsonIconsproperties is a good improvement for code clarity and maintainability.src/core/logical_layers/renderers/stylesConfigs/mcda/types.ts (1)
- 1-4: The update to the
ColorsByMapLibreExpressiontype'sparametersproperty is consistent with the PR's objective to switch from Mapbox GL to MapLibre GL expressions. Ensure that all usages of this type are compatible with the newmaplibregl.Expressiontype.src/features/live_sensor/tests/FakeSensor.ts (4)
12-12: The change from
NodeJS.Timertonumberfor theintervalproperty type aligns with the use ofwindow.setIntervalandwindow.clearInterval, which is appropriate for browser environments.21-23: Using
window.setIntervalensures that the correct function is used in the browser context and avoids potential issues with Node.js types.29-29: Using
window.clearIntervalensures that the correct function is used in the browser context.12-12: Verify if there are any other places in the codebase where the old
NodeJS.Timertype is used and should be updated tonumberto ensure consistency with the changes made here.Verification successful
The executed script searched for occurrences of
NodeJS.Timerin TypeScript files throughout the codebase and did not produce any output. This suggests that there are no remaining references toNodeJS.Timerin TypeScript files, which would imply that the change to usenumber | nullfor theintervalproperty inFakeSensor.tsis consistent with the rest of the codebase.Given this information, it seems that the change to
number | nullis the only instance of this type change, and no further updates are needed to maintain consistency.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the old `NodeJS.Timer` type usage in the codebase. rg 'NodeJS.Timer' --type tsLength of output: 27
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files ignored due to filter (3)
- package-lock.json
- package.json
- pnpm-lock.yaml
Files selected for processing (2)
- scripts/depcheck.js (1 hunks)
- src/core/api_client/apiClient.ts (1 hunks)
Additional comments: 1
src/core/api_client/apiClient.ts (1)
- 4-4: The change from a default import to a named import of
jwtDecodeis consistent with the PR's objective of updating dependencies. Ensure that this change is reflected across the entire codebase whereverjwtDecodeis used.Verification successful
I will correct the regex errors in the shell scripts and rerun them to verify the usage of
jwtDecodeacross the codebase.
To ensure that the change to the
jwtDecodeimport is consistent across the codebase, I will search for any usage ofjwtDecodeto verify that it is not being imported using the old default import pattern elsewhere.
The search results indicate that
jwtDecodeis only used insrc/core/api_client/apiClient.ts, where it is imported with the new named import pattern and used accordingly. There are no instances of the old default import pattern forjwtDecodein the codebase, which suggests that the update to the named import is consistent and complete.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the old default import usage of jwtDecode and ensure it's replaced with the named import. rg "import jwtDecode from 'jwt-decode'" # Search for the new named import usage of jwtDecode to confirm the change is consistent across the codebase. rg "import { jwtDecode } from 'jwt-decode'"Length of output: 229
Script:
#!/bin/bash # Search for the old default import usage of jwtDecode and ensure it's replaced with the named import. rg "import jwtDecode from 'jwt-decode'" # Search for the new named import usage of jwtDecode to confirm the change is consistent across the codebase. rg "import \{ jwtDecode \} from 'jwt-decode'"Length of output: 174
Script:
#!/bin/bash # Search for usage of jwtDecode to ensure it's not being imported with the old pattern anywhere in the codebase. rg "jwtDecode"Length of output: 265
There was a problem hiding this comment.
Review Status
Actionable comments generated: 7
Configuration used: CodeRabbit UI
Files selected for processing (6)
- .github/dependabot.yml (1 hunks)
- .github/workflows/build.yml (3 hunks)
- .github/workflows/deploy_cosmos.yml (1 hunks)
- .github/workflows/size_report.yml (2 hunks)
- .github/workflows/translations_report.yml (1 hunks)
- deployment.js (1 hunks)
Files skipped from review due to trivial changes (2)
- .github/workflows/size_report.yml
- .github/workflows/translations_report.yml
Additional comments: 7
.github/dependabot.yml (1)
- 11-11: The change to a monthly update interval for Dependabot will reduce the frequency of automated dependency updates. Ensure that this aligns with the project's maintenance strategy and that dependencies do not become too outdated, potentially missing important updates or security patches.
.github/workflows/build.yml (3)
47-51: The steps to install dependencies and run scripts have been correctly updated to use
pnpmcommands. This change is consistent with the PR's objective to refresh dependencies and move from npm to pnpm.61-67: The Node.js version has been updated to version 18. Ensure that all project dependencies are compatible with this Node.js version to prevent any potential issues during the build or runtime.
Verification successful
The output from the shell script confirms that the
package.jsonfile specifies that the project requires a Node.js version of>=18. This means that the update to Node.js version 18 in the.github/workflows/build.ymlis compatible with the project's requirements.Final response:
The Node.js version has been updated to version 18 in the CI/CD workflow, which is compatible with the project's requirements as specified in the
package.jsonfile. No issues found with this change.* 64-65: The build job has been updated to use `pnpm ci --include=dev` and `pnpm run build`. These changes are consistent with the switch to pnpm and should work as expected if the project's `pnpm` scripts are correctly defined.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check the `package.json` file for engines.node to verify compatibility with Node.js version 18. rg '"engines":' -A 3 package.jsonLength of output: 89
.github/workflows/deploy_cosmos.yml (1)
- 19-20: The switch from
npmtopnpmcommands in the CI workflow is noted. Ensure that the CI environment haspnpminstalled and that it is compatible with the project's package management setup.deployment.js (2)
15-15: Hardcoded tag value.
94-94: Unclear assignment of
prevCommitSHA.
Akiyamka
force-pushed
the
refresh-deps
branch
from
f368e78 to
49ea776
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (5)
- .github/dependabot.yml (1 hunks)
- .github/workflows/build.yml (4 hunks)
- .github/workflows/deploy_cosmos.yml (1 hunks)
- .github/workflows/size_report.yml (2 hunks)
- .github/workflows/translations_report.yml (1 hunks)
Files skipped from review due to trivial changes (3)
- .github/dependabot.yml
- .github/workflows/deploy_cosmos.yml
- .github/workflows/size_report.yml
Files skipped from review as they are similar to previous changes (2)
- .github/workflows/build.yml
- .github/workflows/translations_report.yml
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- .github/workflows/build.yml (4 hunks)
- .github/workflows/size_report.yml (1 hunks)
- .github/workflows/translations_report.yml (1 hunks)
Files skipped from review due to trivial changes (2)
- .github/workflows/build.yml
- .github/workflows/size_report.yml
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/translations_report.yml
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- .github/workflows/build.yml (4 hunks)
- .github/workflows/size_report.yml (1 hunks)
- .github/workflows/translations_report.yml (1 hunks)
Files skipped from review due to trivial changes (2)
- .github/workflows/size_report.yml
- .github/workflows/translations_report.yml
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build.yml
|
[ |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- package.json
- pnpm-lock.yaml
Files selected for processing (1)
- .eslintrc.json (1 hunks)
Additional comments: 1
.eslintrc.json (1)
- 41-41: Adding the
@typescript-eslint/no-explicit-anyrule with a severity level of "warn" is a good practice to discourage the use of theanytype and promote type safety.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/features/user_profile/components/LoginForm/LoginForm.module.css (1 hunks)
Files skipped from review due to trivial changes (1)
- src/features/user_profile/components/LoginForm/LoginForm.module.css
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- package.json
- pnpm-lock.yaml
Files selected for processing (2)
- src/features/live_sensor/tests/FakeSensor.ts (2 hunks)
- src/utils/storage/index.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- src/utils/storage/index.ts
Files skipped from review as they are similar to previous changes (1)
- src/features/live_sensor/tests/FakeSensor.ts
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #632 +/- ##
=====================================
Coverage 8.21% 8.21%
=====================================
Files 600 599 -1
Lines 40825 40800 -25
Branches 1051 1050 -1
=====================================
- Hits 3354 3353 -1
+ Misses 36950 36927 -23
+ Partials 521 520 -1
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to filter (3)
- devbox.json
- package.json
- pnpm-lock.yaml
Files selected for processing (3)
- .env.development (1 hunks)
- .env.production (1 hunks)
- vite.config.ts (3 hunks)
Files skipped from review due to trivial changes (1)
- .env.production
Additional comments: 2
.env.development (1)
- 5-8: The removal of
VITE_HTTPS=falsefrom the.env.developmentfile is not visible in the hunk but is mentioned in the summary. This change could have significant implications on the development environment's network communication protocol. Ensure that the application's behavior without this environment variable is well understood and documented, and that any necessary adjustments are made to the development setup and documentation.vite.config.ts (1)
- 97-103: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [34-117]
The removal of
VITE_HTTPSfrom the environment configuration and the unconditional enabling of HTTPS in development mode withmkcert()should be double-checked for potential side effects. Ensure that all references toVITE_HTTPSare removed and that the development environment is correctly configured to serve over HTTPS by default.Verification successful
The previous script attempted to use
rgwith an unrecognized file typetypescript. Ripgrep does not have a built-in type for TypeScript, so we should use a wildcard pattern to include.tsand.tsxfiles in our search instead.Let's correct the script to search for any remaining references to
VITE_HTTPSin TypeScript files without specifying the file type.
The updated script did not find any references to
VITE_HTTPSin TypeScript files within the codebase. This suggests that the variable has been successfully removed from the codebase, and there are no lingering references that could cause issues with the configuration changes.Given this information, it appears that the removal of
VITE_HTTPSwas intentional and has been thoroughly applied. The development environment should now be configured to serve over HTTPS by default, as per the changes in thevite.config.tsfile.Since the script execution did not yield any results, and based on the previous context provided, it seems that the removal of
VITE_HTTPShas been handled correctly.The removal of
VITE_HTTPSfrom the environment configuration and the unconditional enabling of HTTPS in development mode withmkcert()have been verified, and no remaining references toVITE_HTTPSwere found in the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to VITE_HTTPS in the codebase. rg --type typescript 'VITE_HTTPS'Length of output: 68
Script:
#!/bin/bash # Search for any remaining references to VITE_HTTPS in TypeScript (.ts and .tsx) files. rg 'VITE_HTTPS' -g '*.ts' -g '*.tsx'Length of output: 36
There was a problem hiding this comment.
Review Status
Actionable comments generated: 5
Configuration used: CodeRabbit UI
Files ignored due to filter (4)
- devbox.json
- devbox.lock
- package.json
- pnpm-lock.yaml
Files selected for processing (1)
- scripts/deployment.mjs (1 hunks)
Additional comments: 2
scripts/deployment.mjs (2)
16-50: The transformation functions for updating
versionandtagin YAML files use regular expressions and replacement logic. Ensure that the regular expressions are robust enough to handle different formatting in the YAML files and consider edge cases where theversionortagmight appear in different contexts within the file.52-55: The
Authorizationheader uses a token directly from the command line arguments. Ensure that this token is handled securely and not exposed in logs or error messages.
| const [version, stageName, build, token] = process.argv.slice(2); | ||
| if (!version) { | ||
| throw Error('You need to pass version'); | ||
| } | ||
|
|
||
| if (!version) { | ||
| throw Error('You need to pass stage'); | ||
| } |
There was a problem hiding this comment.
There seems to be a redundant check for the version variable, which should likely be a check for stageName instead.
- if (!version) {
+ if (!stageName) {
throw Error('You need to pass stage');
}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.
| const [version, stageName, build, token] = process.argv.slice(2); | |
| if (!version) { | |
| throw Error('You need to pass version'); | |
| } | |
| if (!version) { | |
| throw Error('You need to pass stage'); | |
| } | |
| const [version, stageName, build, token] = process.argv.slice(2); | |
| if (!version) { | |
| throw Error('You need to pass version'); | |
| } | |
| if (!stageName) { | |
| throw Error('You need to pass stage'); | |
| } |
| async function createBranchFromMain() { | ||
| // Get the latest commit from the main branch | ||
| const mainBranchData = await fetch( | ||
| `https://api.github.com/repos/${repoOwner}/${repoName}/git/ref/heads/${mainBranch}`, | ||
| { | ||
| headers: headers, | ||
| }, | ||
| ).then((response) => response.json()); | ||
|
|
||
| const latestCommitSha = mainBranchData.object.sha; | ||
|
|
||
| // Create a new branch from the latest commit | ||
| return fetch(`https://api.github.com/repos/${repoOwner}/${repoName}/git/refs`, { | ||
| method: 'POST', | ||
| headers: headers, | ||
| body: JSON.stringify({ | ||
| ref: `refs/heads/${branchName}`, | ||
| sha: latestCommitSha, | ||
| }), | ||
| }); | ||
| } |
There was a problem hiding this comment.
The createBranchFromMain function does not handle potential errors from the GitHub API requests. Implement error handling to catch and respond to issues such as network errors, authentication failures, or invalid responses.
// Add error handling for the fetch request
.then((response) => {
if (!response.ok) {
throw new Error(`GitHub API responded with status: ${response.status}`);
}
return response.json();
})| async function changeConfigAndCommit() { | ||
| let prevCommitSHA; | ||
| const newFiles = await Promise.all( | ||
| Object.keys(transforms).map(async (file) => { | ||
| console.log(` /${file}:`); | ||
| // Retrieve the config file | ||
| const configFileData = await fetch( | ||
| `https://api.github.com/repos/${repoOwner}/${repoName}/contents/${file}?ref=${branchName}`, | ||
| { | ||
| headers: headers, | ||
| }, | ||
| ).then((response) => response.json()); | ||
|
|
||
| prevCommitSHA = configFileData.sha; // TODO: What a correct way to get this? | ||
|
|
||
| // Decode the file content from base64 | ||
| let content = Buffer.from(configFileData.content, 'base64').toString(); | ||
|
|
||
| // Update file content | ||
| content = transforms[file](content); | ||
|
|
||
| // Encode the updated content back to base64 | ||
| const updatedContent = Buffer.from(content, 'utf-8').toString('base64'); | ||
| return { | ||
| path: file, | ||
| mode: '100644', | ||
| type: 'blob', | ||
| content: updatedContent, | ||
| }; | ||
| }), | ||
| ); | ||
|
|
||
| const treeData = await fetch( | ||
| `https://api.github.com/repos/${repoOwner}/${repoName}/git/trees`, | ||
| { | ||
| method: 'POST', | ||
| headers: headers, | ||
| body: JSON.stringify({ | ||
| base_tree: prevCommitSHA, // Base tree of the current branch | ||
| tree: newFiles, | ||
| }), | ||
| }, | ||
| ).then((response) => response.json()); | ||
|
|
||
| // Create a commit with both file changes | ||
| return fetch(`https://api.github.com/repos/${repoOwner}/${repoName}/git/commits`, { | ||
| method: 'POST', | ||
| headers: headers, | ||
| body: JSON.stringify({ | ||
| message: commitMessage, | ||
| tree: treeData.sha, | ||
| parents: [prevCommitSHA], | ||
| }), | ||
| }) | ||
| .then((response) => response.json()) | ||
| .then((commitData) => { | ||
| // Update the reference of the branch to point to the new commit | ||
| return fetch( | ||
| `https://api.github.com/repos/${repoOwner}/${repoName}/git/refs/heads/${branchName}`, | ||
| { | ||
| method: 'PATCH', | ||
| headers: headers, | ||
| body: JSON.stringify({ | ||
| sha: commitData.sha, | ||
| }), | ||
| }, | ||
| ); | ||
| }); | ||
|
|
||
| // // Create a commit with the updated file | ||
| // return fetch(`https://api.github.com/repos/${repoOwner}/${repoName}/contents/${file}`, { | ||
| // method: 'PUT', | ||
| // headers: headers, | ||
| // body: JSON.stringify({ | ||
| // message: commitMessage, | ||
| // content: updatedContent, | ||
| // branch: newBranch, | ||
| // sha: configFileData.sha, | ||
| // }), | ||
| // }); | ||
| } |
There was a problem hiding this comment.
The changeConfigAndCommit function has a TODO comment regarding the correct way to get the previous commit SHA. This needs to be resolved to ensure the script functions correctly. Additionally, the function should handle errors from the GitHub API requests and ensure that the file transformations are applied correctly.
// Resolve the TODO about the correct way to get the previous commit SHA
// Add error handling for the fetch requests| async function createPullRequest() { | ||
| // Open a pull request from the new branch to the main branch | ||
| return fetch(`https://api.github.com/repos/${repoOwner}/${repoName}/pulls`, { | ||
| method: 'POST', | ||
| headers: headers, | ||
| body: JSON.stringify({ | ||
| title: mergeRequestTitle, | ||
| head: branchName, | ||
| base: mainBranch, | ||
| body: 'Pull request created by script.', | ||
| }), | ||
| }); |
There was a problem hiding this comment.
The createPullRequest function does not handle potential errors from the GitHub API requests. Implement error handling to catch and respond to issues such as network errors, authentication failures, or invalid responses.
// Add error handling for the fetch request
.then((response) => {
if (!response.ok) {
throw new Error(`GitHub API responded with status: ${response.status}`);
}
return response.json();
})| async function runScript() { | ||
| await createBranchFromMain(); | ||
| console.log('Update configs:'); | ||
| await changeConfigAndCommit(); | ||
| await createPullRequest(); | ||
| } | ||
|
|
||
| runScript(); |
There was a problem hiding this comment.
The runScript function calls other functions in sequence but does not handle any errors that may occur. Implement error handling to ensure that the script stops execution and reports errors when they occur.
// Add try-catch block to handle errors during script execution
try {
await createBranchFromMain();
console.log('Update configs:');
await changeConfigAndCommit();
await createPullRequest();
} catch (error) {
console.error('Script failed:', error);
process.exit(1);
}
Summary by CodeRabbit
New Features
Bug Fixes
jwtDecodeto ensure proper functionality.Documentation
.npmrcfile to enable pre and post-install scripts for better dependency management.Refactor
mapboxgl.Expressiontomaplibregl.Expressionto align with library updates.isDownloadablelogic for layer source types for clarity and correctness.Style
.errorMessageContainerfor better visual spacing in user interface.Chores
console.debugstatement for cleaner code.Configurations
VITE_HTTPSvariable from development and production environments for simplified configuration management.Security