refactor(angular-query): make injectIsMutating signal read-only #9801
Conversation
π¦ Changeset detectedLatest commit: dd6278b The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Caution Review failedThe pull request is closed. WalkthroughinjectIsMutating now returns a readonly signal view (wrapped with asReadonly()). A changeset documents this patch. The package's tsconfig.json gained an angularCompilerOptions block enabling several strictness flags. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Consumer as Component/Service
participant API as injectIsMutating()
participant Internal as Mutable Signal
note over API,Internal #dff0d8: New behavior β API exposes readonly view
Consumer->>API: call injectIsMutating()
API->>Internal: obtain mutable signal
API-->>Consumer: return asReadonly(Internal) (readonly view)
Consumer->>API: read signal value
API->>Internal: read current value
note right of Internal: Internal may still be mutated by\ninternal query lifecycle (unchanged)
Estimated code review effortπ― 2 (Simple) | β±οΈ ~10 minutes Poem
Pre-merge checks and finishing touchesβ Passed checks (3 passed)
π Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro π Files selected for processing (3)
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 |
|
View your CI Pipeline Execution β for commit dd6278b
βοΈ Nx Cloud last updated this comment at |
Codecov Reportβ
All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9801 +/- ##
===========================================
+ Coverage 45.74% 94.13% +48.39%
===========================================
Files 200 21 -179
Lines 8404 426 -7978
Branches 1920 99 -1821
===========================================
- Hits 3844 401 -3443
+ Misses 4112 24 -4088
+ Partials 448 1 -447 π New features to boost your workflow:
|
![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 (1)
packages/angular-query-experimental/src/inject-is-mutating.ts (1)
22-29: Consider updating JSDoc to document read-only return value.The JSDoc should mention that the returned signal is read-only to help consumers understand the API contract.
Apply this diff to update the JSDoc:
/** * Injects a signal that tracks the number of mutations that your application is fetching. * * Can be used for app-wide loading indicators * @param filters - The filters to apply to the query. * @param options - Additional configuration - * @returns signal with number of fetching mutations. + * @returns A read-only signal with the number of fetching mutations. */
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (3)
.changeset/inject-is-mutating-readonly.md(1 hunks)packages/angular-query-experimental/src/inject-is-mutating.ts(1 hunks)packages/angular-query-experimental/tsconfig.json(1 hunks)
β° Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Preview
π Additional comments (3)
.changeset/inject-is-mutating-readonly.md (1)
1-5: LGTM! Changeset accurately documents the change.The changeset correctly identifies this as a patch release and clearly describes the motivation for making the signal read-only.
packages/angular-query-experimental/src/inject-is-mutating.ts (1)
63-63: LGTM! Proper encapsulation of mutable internal state.The use of
asReadonly()correctly returns a readonly view to external consumers while preserving the internal mutable signal for updates. This prevents accidental external modifications to the mutation count state.packages/angular-query-experimental/tsconfig.json (1)
10-16: <!-- [scratchpad_start]
[task verification_strictness_options 2/2]
git diff returned empty output. Need alternative approach to identify what changed in this PR.[observations]
git diff HEADproduced no output for tsconfig.json- This suggests either: (1) changes not committed, (2) on main branch, or (3) HEAD is the current state
- Review snippet shows the FINAL state (lines 10-16): Angular compiler options with
strictInjectionParameters,strictInputAccessModifiers,strictStandalone,strictTemplatesset to true- Build failed with TypeScript errors (not Angular compiler errors)
- Strictness options shown are in
angularCompilerOptionsblock, not TypeScriptcompilerOptions[analysis]
- Review comment focuses on Angular strictness options, but errors are TypeScript compile errors
- Need to determine: are these options NEW in this PR, or were they already there?
- Need to check git log or PR diff to see what actually changed
[pending]
- Check git log to see recent changes to this file
- Or check if there's a PR base branch to compare against
- Or directly view tsconfig.json to see all options
[actions]
Try git log to see recent commits, or check git status and branches to understand context
arnoud-dv
force-pushed
the
refactor/inject-is-mutating-readonly-signal
branch
from
9bfee6e to
dd6278b
Compare
1 participant
π― Changes
The signal returned by injectIsMutating should be read-only as it is not supposed to be changed outside the library.
β Checklist
pnpm run test:pr.π Release Impact
Summary by CodeRabbit
Refactor
Chores
Documentation