Answer:16 dependency injection#1525
Conversation
github-actions
Bot
added
16
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🤖 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 `@apps/angular/16-master-dependency-injection/src/app/app.component.ts`:
- Around line 32-33: The row directive selector in AppComponent does not follow
the Angular ESLint directive-selector convention, so update the `@Directive`
selector from the current attribute name to an app-prefixed one and keep the
template usage in sync. Fix the selector on the directive class in
app.component.ts (the directive that handles currency code rows) and update the
matching binding wherever it is used in the template so the selector and markup
stay aligned; if the selector must remain unchanged, add a local lint disable
instead.
- Around line 40-42: The unused private EffectRef field in AppComponent should
be removed because it is only created and never read or destroyed. Update the
effect setup in the app component so the sync logic still runs via effect(() =>
...) but do not assign it to `#syncCodeEffectRef`, and keep the patchState call in
the effect body unchanged.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2428df75-fe66-420a-b5a1-9d2456b3e53b
📒 Files selected for processing (2)
apps/angular/16-master-dependency-injection/src/app/app.component.tslibs/shared/ui/src/lib/table.component.ts
| @Directive({ | ||
| selector: 'tr[currencyCode]', |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== File outline ==\n'
ast-grep outline apps/angular/16-master-dependency-injection/src/app/app.component.ts --view expanded || true
printf '\n== Relevant file excerpt ==\n'
cat -n apps/angular/16-master-dependency-injection/src/app/app.component.ts | sed -n '1,140p'
printf '\n== Search for directive selector lint config ==\n'
rg -n "`@angular-eslint/directive-selector`|directive-selector" -S . --glob '!**/node_modules/**' --glob '!**/dist/**' || true
printf '\n== Search for inline disables near the directive ==\n'
rg -n "eslint-disable|directive-selector|currencyCode" apps/angular/16-master-dependency-injection/src/app/app.component.ts -n -S || trueRepository: tomalaforge/angular-challenges
Length of output: 11889
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n apps/angular/16-master-dependency-injection/eslint.config.mjsRepository: tomalaforge/angular-challenges
Length of output: 1098
Prefix the row directive selector to match the lint rule.
@angular-eslint/directive-selector here requires an app-prefixed attribute selector, so tr[currencyCode] will fail lint. Rename it to something like tr[appCurrencyCode] and update the template binding at line 66, or add a local disable if this exact selector is intentional.
🧰 Tools
🪛 ESLint
[error] 33-33: The selector should start with one of these prefixes: "app"
(@angular-eslint/directive-selector)
🤖 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 `@apps/angular/16-master-dependency-injection/src/app/app.component.ts` around
lines 32 - 33, The row directive selector in AppComponent does not follow the
Angular ESLint directive-selector convention, so update the `@Directive` selector
from the current attribute name to an app-prefixed one and keep the template
usage in sync. Fix the selector on the directive class in app.component.ts (the
directive that handles currency code rows) and update the matching binding
wherever it is used in the template so the selector and markup stay aligned; if
the selector must remain unchanged, add a local lint disable instead.
Source: Linters/SAST tools
| #syncCodeEffectRef = effect(() => { | ||
| this.#currencyService.patchState({ code: this.currencyCode() }); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
git ls-files apps/angular/16-master-dependency-injection/src/app/app.component.ts && printf '\n---\n' && cat -n apps/angular/16-master-dependency-injection/src/app/app.component.tsRepository: tomalaforge/angular-challenges
Length of output: 2772
🏁 Script executed:
rg -n "no-unused-private-class-members|EffectRef|effect\(" apps/angular/16-master-dependency-injection/src/app -SRepository: tomalaforge/angular-challenges
Length of output: 278
Drop the unused EffectRef field. #syncCodeEffectRef is never read or destroyed, so it just trips no-unused-private-class-members. If you only need the side effect, create the effect without storing the ref.
🧰 Tools
🪛 ESLint
[error] 40-40: '#syncCodeEffectRef' is defined but never used.
(no-unused-private-class-members)
🤖 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 `@apps/angular/16-master-dependency-injection/src/app/app.component.ts` around
lines 40 - 42, The unused private EffectRef field in AppComponent should be
removed because it is only created and never read or destroyed. Update the
effect setup in the app component so the sync logic still runs via effect(() =>
...) but do not assign it to `#syncCodeEffectRef`, and keep the patchState call in
the effect body unchanged.
Source: Linters/SAST tools
1 participant
✅ Challenge Submission Checklist
Start your PR title with: Answer:${challenge_number}
If you would like personal feedback or a detailed review, please support the project on GitHub:
👉 https://github.com/sponsors/tomalaforge
You can also submit a PR without sponsorship to:
Summary by CodeRabbit
New Features
Bug Fixes