fix: misleading dual nativescript.config warning when reading plugin configs - #6122
fix: misleading dual nativescript.config warning when reading plugin configs#6122NathanWalker wants to merge 1 commit into
Conversation
…configs During iOS prepare, plugin-declared SPM packages are collected by reading each plugin's nativescript.config.ts via projectConfigService.readConfig. A plugin that publishes compiled artifacts next to its config source triggered the project-level "You have both a nativescript.config.js and nativescript.config.ts file" warning, with no indication it referred to a directory inside node_modules rather than the user's project. Plugin config reads now suppress the warning, and the warning message names the directory it refers to for genuine dual-config projects.
📝 WalkthroughWalkthroughThe project configuration APIs now accept optional warning suppression. The service skips dual-config warnings when requested and includes the configuration directory in the warning message. iOS plugin configuration loading enables suppression. ChangesConfiguration warning suppression
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant IOSProjectService
participant ProjectConfigService
IOSProjectService->>ProjectConfigService: readConfig(pluginDirectory, suppressWarnings=true)
ProjectConfigService->>ProjectConfigService: detectProjectConfigs(suppressWarnings=true)
ProjectConfigService-->>IOSProjectService: plugin configuration without dual-config warning
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/services/project-config-service.ts`:
- Around line 160-164: Update the warning in the configuration-detection branch
of the project config service to report the actual detected JSConfigPath and
TSConfigPath values, rather than hardcoded default names and only TSConfigPath’s
directory. Preserve the warning’s statement that TypeScript configuration is
selected, and ensure any directory-based wording is used only when both paths
share the same parent directory.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 995860e9-e84d-4f6b-853c-a5496df365c3
📒 Files selected for processing (3)
lib/definitions/project.d.tslib/services/ios-project-service.tslib/services/project-config-service.ts
| if (hasTSConfig && hasJSConfig && !options?.suppressWarnings) { | ||
| this.$logger.warn( | ||
| `You have both a ${CONFIG_FILE_NAME_JS} and ${CONFIG_FILE_NAME_TS} file. Defaulting to ${CONFIG_FILE_NAME_TS}.`, | ||
| `You have both a ${CONFIG_FILE_NAME_JS} and ${CONFIG_FILE_NAME_TS} file in ${path.dirname( | ||
| TSConfigPath, | ||
| )}. Defaulting to ${CONFIG_FILE_NAME_TS}.`, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Report the detected configuration paths.
possibleConfigPaths can combine a custom --config or NATIVESCRIPT_CONFIG_NAME path with the default paths. Therefore, TSConfigPath and JSConfigPath are not guaranteed to have the same parent directory. This message can claim that both files are in path.dirname(TSConfigPath) when the JavaScript file is elsewhere. It also names the default files when custom names were detected.
Build the warning from JSConfigPath and TSConfigPath, or use the directory form only when both parent directories match.
Proposed fix
this.$logger.warn(
- `You have both a ${CONFIG_FILE_NAME_JS} and ${CONFIG_FILE_NAME_TS} file in ${path.dirname(
- TSConfigPath,
- )}. Defaulting to ${CONFIG_FILE_NAME_TS}.`,
+ `You have both ${JSConfigPath} and ${TSConfigPath}. Defaulting to ${TSConfigPath}.`,
);📝 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.
| if (hasTSConfig && hasJSConfig && !options?.suppressWarnings) { | |
| this.$logger.warn( | |
| `You have both a ${CONFIG_FILE_NAME_JS} and ${CONFIG_FILE_NAME_TS} file. Defaulting to ${CONFIG_FILE_NAME_TS}.`, | |
| `You have both a ${CONFIG_FILE_NAME_JS} and ${CONFIG_FILE_NAME_TS} file in ${path.dirname( | |
| TSConfigPath, | |
| )}. Defaulting to ${CONFIG_FILE_NAME_TS}.`, | |
| if (hasTSConfig && hasJSConfig && !options?.suppressWarnings) { | |
| this.$logger.warn( | |
| `You have both ${JSConfigPath} and ${TSConfigPath}. Defaulting to ${TSConfigPath}.`, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/services/project-config-service.ts` around lines 160 - 164, Update the
warning in the configuration-detection branch of the project config service to
report the actual detected JSConfigPath and TSConfigPath values, rather than
hardcoded default names and only TSConfigPath’s directory. Preserve the
warning’s statement that TypeScript configuration is selected, and ensure any
directory-based wording is used only when both paths share the same parent
directory.
1 participant
During iOS prepare, plugin-declared SPM packages are collected by reading each plugin's nativescript.config.ts via projectConfigService.readConfig.
A plugin that publishes compiled artifacts next to its config source triggered the project-level:
warning, with no indication it referred to a directory inside node_modules rather than the user's project.
Plugin config reads now suppress the warning, and the warning message names the directory it refers to for genuine dual-config projects.
Summary by CodeRabbit
Bug Fixes
Improvements