vb-branch-1 #339
vb-branch-1 #339
Conversation
Deploying flyxc with
|
| Latest commit: |
a65300a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3979dcd0.flyxc.pages.dev |
| Branch Preview URL: | https://vb-branch-1.flyxc.pages.dev |
Reviewer's Guide by SourceryThis PR upgrades Nx to 20.3.0 and ESLint to 9.8.0, along with several dependency updates and minor code fixes. Class diagram showing updated ESLint configuration structureclassDiagram
class BaseESLintConfig {
+plugins
+rules
+files
+ignores
}
class ModuleConfig {
+extends BaseConfig
+customRules
+moduleSpecificIgnores
}
class ESLintPlugins {
+@nx/eslint-plugin
+require-node-import-prefix
+simple-import-sort
+import
}
BaseESLintConfig <|-- ModuleConfig
BaseESLintConfig o-- ESLintPlugins
note for BaseESLintConfig "New flat config format"
note for ModuleConfig "Per-module configuration"
note for ESLintPlugins "Updated plugin set"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThis pull request introduces a comprehensive update to the project's ESLint configuration strategy. The changes involve replacing legacy Changes
Poem
Possibly Related PRs
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @vicb - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (13)
libs/windy-sounding/vite.config.ts (1)
8-9: Consider more secure handling of certificates and private keys.Storing certificates and private keys in your code repository can introduce security risks if these files are misused or inadvertently exposed in production environments. If these certificates are only for local development usage, documenting them as such and ensuring they are never deployed to production is important. Alternatively, consider referencing environment variables or external secure storage services (e.g., Vault) to manage sensitive data more securely.
Would you like assistance setting up environment-based configuration or referencing a secure secret management solution?
eslint.config.js (3)
13-20: Use Additional Plugin Descriptions.You're specifying multiple plugins under
plugins, but you haven't documented their intent. Including a brief rationale for each plugin can help maintainers understand why they're needed.
54-55: Avoid Duplicateno-extra-semiDeclarations.You set
"no-extra-semi": "error"in two places: lines 54 and 69. Consider consolidating them to simplify maintenance.- 'no-extra-semi': 'error', ... - 'no-extra-semi': 'error', + // Remove one of the duplicate entriesAlso applies to: 69-69
72-74: Ensure Ignored Paths List is Comprehensive.You’re ignoring build-related directories but might also consider ignoring other artifact folders (e.g.,
dist,coverage) if you have them elsewhere in the repo.apps/fxc-front/eslint.config.js (1)
3-7: Complement Base Config with Additional Rules if Needed.You’ve extended the base configuration and added ignores. This is fine. However, if
fxc-fronthas unique complexities (browser-specific code, frameworks, etc.), consider adding any specialized lint rules to preempt framework-specific pitfalls.libs/secrets/eslint.config.js (1)
10-12: Check JSON Parser Impact on Build or Tooling.Switching to
jsonc-eslint-parseris helpful, especially for comments in JSON. Confirm that other tools that rely on JSON files are unaffected by the extended JSON syntax.apps/fetcher/eslint.config.js (1)
6-19: Refine Rule Overrides for Different Environments.You have placeholders for TypeScript and JavaScript rule overrides. Ensure each environment has rules relevant to your actual usage (e.g., Node vs. browser, test vs. production).
libs/optimizer/eslint.config.cjs (1)
9-16: Assess Strictness ofimport/no-extraneous-dependencies.Prohibiting all dev/optional/peer dependencies in
.ts/.jsfiles can be overly strict for testing. If you have test files in the same directory, consider allowing devDependencies for test files.'import/no-extraneous-dependencies': [ 'error', { - devDependencies: false, + devDependencies: ['**/*.spec.ts', '**/*.test.ts', '**/*.spec.js', '**/*.test.js'], optionalDependencies: false, peerDependencies: false }, ]libs/windy-sounding/eslint.config.cjs (2)
6-17: Confirm devDependencies usage within test files
You have disableddevDependenciesin'import/no-extraneous-dependencies'. This rule may raise false positives for test files that legitimately rely on testing libraries. If you intend to allow dev dependencies in test code, consider adding a separate override for test files.
34-37: Consider unifying or removing ignore patterns
While ignoringnode_modules/is standard, confirm whether ignoringgenerate-manifest.js,vite.config.ts, andtypes/is necessary. If you intend to lint any of these files, move them to an override block instead of ignoring them completely.libs/windy-sounding/package.json (1)
23-23: Pinningjsonc-eslint-parserto 2.4.0
Pinning dependencies can limit unwanted updates but may also miss essential bug fixes or security patches. Consider using a caret version range (e.g.,^2.4.0) to allow minor and patch updates, if compatible with your project.libs/windy-sounding/generate-manifest.js (2)
Line range hint
5-12: Optional: Switch to asynchronous file handling for better performanceWhile
fs.writeFileSyncis simpler for small scripts, consider using non-blocking I/O (fs.promises) if you anticipate additional logic or expansions in this script.- fs.writeFileSync(manifestPath, manifest); + await fs.promises.writeFile(manifestPath, manifest);
Line range hint
5-12: Surface more debugging detailsWhen catching errors, you might want to include stack traces or additional metadata to expedite troubleshooting. As an example:
} catch (error) { - console.error(`Error generating manifest file: ${error.message}`); + console.error(`Error generating manifest file at ${manifestPath}: ${error.stack}`); process.exit(1); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
libs/secrets/package-lock.jsonis excluded by!**/package-lock.jsonlibs/windy-sounding/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (70)
.eslintignore(0 hunks).eslintrc.json(0 hunks).gitignore(1 hunks)apps/fetcher/.eslintrc.json(0 hunks)apps/fetcher/eslint.config.js(1 hunks)apps/fetcher/jest.config.ts(0 hunks)apps/fetcher/src/app/trackers/flymaster.test.ts(0 hunks)apps/fetcher/src/app/trackers/inreach.test.ts(0 hunks)apps/fetcher/src/app/trackers/proxies.ts(0 hunks)apps/fetcher/src/app/trackers/skylines.test.ts(0 hunks)apps/fetcher/src/app/trackers/spot.test.ts(0 hunks)apps/fetcher/src/app/trackers/xcontest.test.ts(0 hunks)apps/fetcher/src/app/trackers/xcontest.ts(0 hunks)apps/fxc-front/.eslintrc.json(0 hunks)apps/fxc-front/eslint.config.js(1 hunks)apps/fxc-front/jest.config.ts(0 hunks)apps/fxc-front/src/app/components/3d/airspace3d-element.ts(1 hunks)apps/fxc-front/src/app/components/3d/skyways3d-element.ts(1 hunks)apps/fxc-front/src/app/components/ui/ion-router.ts(1 hunks)apps/fxc-front/src/app/logic/airspaces.ts(0 hunks)apps/fxc-server/.eslintrc.json(0 hunks)apps/fxc-server/eslint.config.js(1 hunks)apps/fxc-server/jest.config.ts(0 hunks)apps/fxc-server/src/app/routes/supporters.ts(1 hunks)apps/fxc-server/src/app/waypoints/waypoints.ts(0 hunks)apps/fxc-server/src/main.ts(0 hunks)apps/fxc-tiles/.eslintrc.json(0 hunks)apps/fxc-tiles/eslint.config.js(1 hunks)apps/fxc-tiles/jest.config.ts(0 hunks)apps/fxc-tiles/src/app/airspaces/create-geojson.ts(1 hunks)apps/misc/.eslintrc.json(0 hunks)apps/misc/eslint.config.js(1 hunks)apps/misc/jest.config.ts(0 hunks)apps/proxy/.eslintrc.json(0 hunks)apps/proxy/eslint.config.js(1 hunks)apps/proxy/jest.config.ts(0 hunks)apps/run/.eslintrc.json(0 hunks)apps/run/eslint.config.js(1 hunks)apps/run/jest-bench.config.ts(0 hunks)apps/run/jest.config.ts(0 hunks)eslint.config.js(1 hunks)libs/common-node/.eslintrc.json(0 hunks)libs/common-node/eslint.config.js(1 hunks)libs/common-node/jest.config.ts(0 hunks)libs/common/.eslintrc.json(0 hunks)libs/common/eslint.config.js(1 hunks)libs/common/jest.config.ts(0 hunks)libs/common/src/lib/fetch-timeout.ts(1 hunks)libs/optimizer/.eslintrc.json(0 hunks)libs/optimizer/eslint.config.cjs(1 hunks)libs/optimizer/jest.config.ts(0 hunks)libs/optimizer/package.json(1 hunks)libs/secrets/.eslintrc.json(0 hunks)libs/secrets/eslint.config.js(1 hunks)libs/secrets/package.json(1 hunks)libs/vaadin-dom/.eslintrc.json(0 hunks)libs/vaadin-dom/eslint.config.js(1 hunks)libs/vaadin-dom/jest.config.ts(0 hunks)libs/vaadin-dom/src/lib/Field.ts(0 hunks)libs/vaadin-nodom/.eslintrc.json(0 hunks)libs/vaadin-nodom/eslint.config.js(1 hunks)libs/windy-sounding/.eslintrc.json(0 hunks)libs/windy-sounding/eslint.config.cjs(1 hunks)libs/windy-sounding/generate-manifest.js(1 hunks)libs/windy-sounding/jest.config.ts(0 hunks)libs/windy-sounding/package.json(1 hunks)libs/windy-sounding/src/redux/forecast-slice.ts(1 hunks)libs/windy-sounding/vite.config.ts(1 hunks)nx.json(2 hunks)package.json(4 hunks)
💤 Files with no reviewable changes (40)
- libs/optimizer/jest.config.ts
- apps/fxc-tiles/jest.config.ts
- apps/fxc-front/jest.config.ts
- libs/vaadin-dom/jest.config.ts
- libs/common/jest.config.ts
- apps/proxy/jest.config.ts
- apps/fetcher/jest.config.ts
- .eslintignore
- apps/misc/jest.config.ts
- apps/fetcher/src/app/trackers/flymaster.test.ts
- apps/run/jest-bench.config.ts
- libs/windy-sounding/jest.config.ts
- apps/fetcher/src/app/trackers/skylines.test.ts
- apps/fxc-server/src/app/waypoints/waypoints.ts
- apps/fxc-server/jest.config.ts
- apps/fetcher/src/app/trackers/inreach.test.ts
- apps/run/jest.config.ts
- libs/vaadin-dom/.eslintrc.json
- libs/vaadin-nodom/.eslintrc.json
- libs/common/.eslintrc.json
- apps/fetcher/src/app/trackers/xcontest.test.ts
- apps/proxy/.eslintrc.json
- apps/fxc-tiles/.eslintrc.json
- libs/common-node/.eslintrc.json
- apps/fetcher/.eslintrc.json
- libs/secrets/.eslintrc.json
- apps/fxc-server/src/main.ts
- apps/fetcher/src/app/trackers/spot.test.ts
- apps/fetcher/src/app/trackers/proxies.ts
- libs/common-node/jest.config.ts
- apps/run/.eslintrc.json
- .eslintrc.json
- apps/fxc-server/.eslintrc.json
- apps/fxc-front/src/app/logic/airspaces.ts
- apps/misc/.eslintrc.json
- apps/fetcher/src/app/trackers/xcontest.ts
- apps/fxc-front/.eslintrc.json
- libs/windy-sounding/.eslintrc.json
- libs/optimizer/.eslintrc.json
- libs/vaadin-dom/src/lib/Field.ts
✅ Files skipped from review due to trivial changes (16)
- libs/vaadin-nodom/eslint.config.js
- apps/fxc-tiles/src/app/airspaces/create-geojson.ts
- apps/proxy/eslint.config.js
- apps/fxc-server/eslint.config.js
- apps/run/eslint.config.js
- libs/secrets/package.json
- apps/misc/eslint.config.js
- apps/fxc-tiles/eslint.config.js
- apps/fxc-server/src/app/routes/supporters.ts
- libs/common/eslint.config.js
- libs/vaadin-dom/eslint.config.js
- libs/common-node/eslint.config.js
- libs/common/src/lib/fetch-timeout.ts
- libs/optimizer/package.json
- apps/fxc-front/src/app/components/ui/ion-router.ts
- libs/windy-sounding/src/redux/forecast-slice.ts
🔇 Additional comments (18)
apps/fxc-front/src/app/components/3d/skyways3d-element.ts (1)
73-73: Good move to properly assign undefined after removing and destroying the layer.
This ensures there's no lingering reference to the destroyed layer, preventing potential memory leaks or accidental reuse.
apps/fxc-front/src/app/components/3d/airspace3d-element.ts (1)
89-89: Ensuring the layer is explicitly set to undefined helps avoid stale references.
This aligns with best practices for cleaning up child objects, especially after a destroy() call.
eslint.config.js (2)
25-37: Verify Nx Enforce Module Boundaries Configuration.
The @nx/enforce-module-boundaries rule calls out allow: ['@windy']. Ensure that the library or path @windy is indeed valid within your workspace, and confirm that depending on it doesn't violate Nx constraints.
45-53: Confirm the Disabled no-unused-vars Rule.
Disabling the built-in no-unused-vars in favor of @typescript-eslint/no-unused-vars is generally a good practice. However, double-check that there are no edge cases in plain JavaScript files outside TypeScript coverage.
libs/secrets/eslint.config.js (1)
6-9: Validate @nx/dependency-checks for JSON Files.
Enforcing @nx/dependency-checks on JSON files is unusual. Verify that you truly need to lint dependencies within JSON-based configurations (e.g., package.json). If it’s unintentional, consider refining the pattern.
libs/optimizer/eslint.config.cjs (2)
18-18: Confirm Spec Files Are Patched.
You're ignoring **/*.spec.ts. If you have test specs for .js or .jsx files, ensure they are also excluded if that’s your intent.
31-37: Apply Parser Only Where Necessary.
You’re parsing JSON files with jsonc-eslint-parser to highlight issues or allow comments. Ensure that other file types remain unaffected, especially if you rely on standard JSON parsing in certain contexts.
libs/windy-sounding/eslint.config.cjs (4)
1-2: Use consistent extension for the base config import.
Since this new file is a CommonJS module (eslint.config.cjs), ensure that ../../eslint.config.js is compatible with CommonJS imports or rename it to .cjs for consistency.
3-4: Exporting an array is valid in ESLint's flat config
The flat config format supports exporting an array of configuration objects. This approach provides fine-grained control over file pattern matching and rule application.
16-16: Check restricted import
The rule 'no-restricted-imports': ['error', '@windycom'] effectively bans all imports from @windycom. Verify that you don’t need partial or internal sub-path imports.
19-33: Efficient JSON linting using jsonc-eslint-parser
Your configuration for JSON files looks solid, especially ignoring certain dependencies for Nx checks. This approach helps maintain consistent dependency usage. Confirm that ignoring 'preact' or 'react-redux' is intentional.
.gitignore (1)
48-49: Ensure test configuration files are consistently ignored
By ignoring vite.config.*.timestamp* and vitest.config.*.timestamp*, you are covering ephemeral or build-related files. Confirm that no local development or testing workflows rely on these files.
nx.json (2)
23-23: Include new ESLint config
Referencing "{workspaceRoot}/eslint.config.js" ensures Nx picks up the new flat ESLint config. Confirm no references to removed .eslintrc.json files remain.
49-50: Exclude ESLint config in production
Excluding "{projectRoot}/eslint.config.js" from production is common practice, but verify that no production logic depends on this file (e.g., custom build steps).
package.json (3)
97-97: Verify security implications of dependency updates
Notable updates to security-sensitive packages:
@google-cloud/computefrom ^4.8.0 to ^4.9.0ioredisfrom ^5.4.1 to ^5.4.2
These packages handle sensitive operations (cloud infrastructure, data storage).
Run this script to check for known vulnerabilities:
Also applies to: 136-136
✅ Verification successful
Dependencies updated to secure versions with no known vulnerabilities
The GitHub Security Advisory Database shows no known vulnerabilities for either @google-cloud/[email protected] or [email protected]. These minor version updates appear to be safe to use.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for known vulnerabilities in updated packages
# Check for security advisories for google-cloud-compute
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "@google-cloud/compute") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
# Check for security advisories for ioredis
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "ioredis") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 694
22-32: Verify Nx framework version compatibility
All Nx packages have been updated from 20.2.2 to 20.3.0. While this is a minor version bump, it's important to verify compatibility with your project configuration.
Run this script to check for potential breaking changes:
Also applies to: 73-73
✅ Verification successful
Nx configuration is compatible with version 20.3.0
Based on the analysis of the project configuration files, the upgrade from Nx 20.2.2 to 20.3.0 is safe as:
- All projects use standard Nx executors (
@nx/webpack,@nx/vite,@nx/js) without custom implementations - The project structure follows Nx's recommended patterns with proper schema references
- No deprecated features or executors are being used
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check Nx configuration compatibility
# Check for Nx workspace configuration
echo "Checking Nx workspace config..."
fd -g 'nx.json|workspace.json'
# Look for custom Nx executors/generators that might need updates
echo "Checking for custom Nx implementations..."
fd -g 'project.json' | xargs cat
Length of output: 21756
18-18: Verify ESLint major version upgrade compatibility
The ESLint-related changes include significant updates:
- New dependency
@eslint/eslintrc@^3.2.0 - Major version bump of
eslintfrom ~8.57.0 to ^9.8.0 - New dependency
typescript-eslint@^8.13.0
These changes align with modernizing the ESLint configuration, but the major version bump of ESLint requires careful verification.
Run this script to check for potential breaking changes:
Also applies to: 60-60, 82-82
✅ Verification successful
Let me verify the ESLint configuration by checking one of the new config files to ensure it's properly set up for ESLint v9.
ESLint v9 configuration is properly set up with flat config
The verification shows that:
- All
.eslintrc.*files have been removed - New
eslint.config.jsfiles are present in the root and all apps/libs - The configuration uses the new flat config format with
@eslint/eslintrc'sFlatCompat - ESLint directives in source files are using standard rules that are compatible with v9
- The root config properly extends and configures all necessary plugins
- App-specific configs correctly extend the root config
The ESLint-related dependency changes are verified to be working correctly with the new flat config system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for ESLint v9 breaking changes and verify config compatibility
# Search for ESLint configuration files
echo "Searching for ESLint config files..."
fd -e js -e cjs -e mjs -g 'eslint.config.*'
# Search for any remaining .eslintrc files that might conflict
echo "Checking for legacy ESLint config files..."
fd -g '.eslintrc*'
# Look for ESLint directives in source files that might need updates
echo "Checking for ESLint directives in source files..."
rg "eslint-disable|eslint-enable|eslint-env"
Length of output: 3415
Script:
#!/bin/bash
# Check the root eslint.config.js and a sample app config
echo "=== Root ESLint config ==="
cat eslint.config.js
echo -e "\n=== Sample app ESLint config ==="
cat apps/fetcher/eslint.config.js
Length of output: 2950
libs/windy-sounding/generate-manifest.js (1)
3-3: Use caution when importing from the dist folder
Referencing the ../../dist/libs/windy-sounding/config.js bundle can break if the build output moves or changes in name. Consider importing configuration directly from the source if feasible, or confirming the build process consistently outputs the file at this path.
Run this script to confirm there are no references to a config.js in unexpected build directories:
✅ Verification successful
Let me gather more information about the config.js usage in the windy-sounding library.
Let me check for TypeScript config files since the project seems to use TypeScript.
Let me check the build configuration and the actual config.ts content to understand how it's being built.
The import from dist/libs/windy-sounding/config.js is intentional and correct
The import from the dist folder is actually the intended behavior in this case. Looking at the build configuration:
- The project has a special build step that generates the config bundle:
BUILD_PLUGIN_CONFIG=true npx nx build windy-sounding --configuration production- This creates a separate config.js bundle using a specific vite configuration
- The generate-manifest.js script is designed to run after the build and specifically needs the built config
This is a deliberate architectural decision where:
- The config is built separately from the main plugin
- The manifest generator uses the built config to ensure consistency with the deployed version
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching for config.js references outside of dist directory.
rg "config\.js" -A 3
Length of output: 12139
Script:
#!/bin/bash
# Check for config.js in the windy-sounding library
fd "config.js" libs/windy-sounding/
# Check the content of the config file in source
cat libs/windy-sounding/src/config.js || true
# Look for build configuration that handles config.js
rg -l "config\.js" libs/windy-sounding/vite.config.ts libs/windy-sounding/project.json
Length of output: 308
Script:
#!/bin/bash
# Check for TypeScript config files
fd "config.ts" libs/windy-sounding/src/
# Look for imports of config in the codebase
rg -l "from.*config" libs/windy-sounding/src/
Length of output: 280
Script:
#!/bin/bash
# Check the content of config.ts
cat libs/windy-sounding/src/config.ts
# Check build configuration
cat libs/windy-sounding/project.json
# Check vite config
cat libs/windy-sounding/vite.config.ts || true
Length of output: 5644
Summary by Sourcery
Upgrade Nx to 20.3.0 and ESLint to 9.8.0.
Build:
Tests:
Chores:
Summary by CodeRabbit
Based on the comprehensive summary of changes, here are the release notes:
ESLint Configuration
.eslintrc.jsontoeslint.config.jsacross multiple projectsDependency Updates
@nxpackages from version20.2.2to20.3.09.8.0@eslint/eslintrcandtypescript-eslintMinor Code Improvements
Package Management