Skip to content

Cleanup#100

Merged
alexanderharding merged 9 commits into
mainfrom
cleanup
May 21, 2026
Merged

Cleanup#100
alexanderharding merged 9 commits into
mainfrom
cleanup

Conversation

@alexanderharding

@alexanderharding alexanderharding commented May 21, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • New Features

    • share() now manages source AbortSignals to better control forwarded subscriptions.
  • Bug Fixes

    • Stricter argument validation and clearer error messages for reduce() and scan().
    • Fixed switch-map documentation example and link formatting.
    • Corrected error assertion behavior in last-value-from tests.
  • Tests

    • Expanded argument-validation and AbortSignal coverage; clarified several test descriptions.
  • Chores

    • Bumped versions for pairwise, reduce, scan, share, and switch-map.

Review Change Stack

@alexanderharding alexanderharding self-assigned this May 21, 2026
@coderabbitai

coderabbitai Bot commented May 21, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3c7f8662-3523-456c-86e6-73fbe44bcb31

📥 Commits

Reviewing files that changed from the base of the PR and between 76618d1 and 7752e6a.

📒 Files selected for processing (1)
  • last-value-from/mod.test.ts

📝 Walkthrough

Walkthrough

This PR consolidates operator improvements across the observable library: stricter argument validation in reduce and scan requiring exactly two parameters, introduction of internal AbortController lifecycle management in share() for cleaner teardown, corrections to test assertions and descriptions across multiple operators, and refinements to TypeScript type guards and documentation examples.

Changes

Observable operator improvements

Layer / File(s) Summary
Argument validation enforcement in reduce
reduce/mod.ts, reduce/mod.test.ts, reduce/deno.json
reduce() now enforces exactly two required arguments (reducer and initialValue), throwing TypeError with updated messages when called with zero or one argument. Test coverage validates both missing-argument and invalid-type scenarios.
Argument validation enforcement in scan
scan/mod.ts, scan/mod.test.ts, scan/deno.json
scan() argument validation is tightened to require exactly two arguments, replacing the prior "1 argument required" contract with "2 arguments required" error messages. New test cases validate missing and invalid argument types.
AbortSignal lifecycle in share subscription
share/mod.ts, share/mod.test.ts, share/deno.json
share() now manages an internal sourceController (AbortController) that unifies termination signals from both the subject and the source subscription cleanup path. When all observers unsubscribe, the source is aborted and references are cleared; fresh controllers are created on each reset cycle.
Test assertion corrections and description refinements
empty/mod.test.ts, finalize/mod.test.ts, from/mod.test.ts, interval/mod.test.ts, last-value-from/mod.test.ts, tap/mod.test.ts
Multiple operator test suites receive corrected assertions and clarified test descriptions: empty test uses signal-aware Observer options, finalize tests add argument validation, last-value-from fixes shadowing error variable capture, and test names are refined to better describe what is being validated.
Type guard refinement and documentation examples
pairwise/mod.ts, pairwise/deno.json, switch-map/mod.ts, switch-map/README.md, switch-map/deno.json
pairwise() filter predicate is updated with explicit TypeScript type guard syntax to narrow emitted values, while switch-map documentation examples and README are corrected to use the emitted value variable instead of an incorrect page reference.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A rabbit hops through validation, hop hop hop,
Checking arguments before they stop,
AbortSignals dance in share's new flow,
Tests spring clean where corrections grow,
Type guards guard and docs align—refined, refined! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Cleanup' is vague and generic, using a non-descriptive term that fails to convey meaningful information about the specific changes made across the 18 modified files and their purposes. Replace with a more specific title that highlights the primary change, such as 'Add argument validation tests and improve type safety' or 'Enhance error handling and fix documentation across operators'.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cleanup

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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 `@last-value-from/mod.test.ts`:
- Around line 13-18: The test for lastValueFrom(throwError(error)) uses a
try/catch without asserting failure on the success path, which can produce a
false-positive; update the test to either use assertRejects(() =>
lastValueFrom(throwError(error))) or keep the try/catch but add a fail() (or
throw) immediately after the await to ensure the test fails if no rejection
occurs, referencing the lastValueFrom and throwError calls in the test.

In `@reduce/mod.test.ts`:
- Around line 11-57: Refactor each Deno.test in reduce/mod.test.ts to follow
Arrange/Act/Assert: for tests calling reduce or the returned operatorFn
(symbols: reduce, operatorFn) create an explicit Arrange section that sets up
inputs and any stubs, an Act section that performs the call (e.g., capture the
function that will throw), and an Assert section that calls assertThrows with
the prepared act function and expected error/type/message; do this for the
zero-arg, one-arg, non-function reducer, missing initial value, and
non-Observable source tests so each test body clearly separates setup, action,
and assertion.

In `@scan/mod.test.ts`:
- Around line 13-58: Refactor each failing validation test to follow
Arrange/Act/Assert: for each Deno.test, first ARRANGE any inputs/mocks (e.g.,
prepare reducer or operatorFn via scan), then ACT by invoking the function under
test inside a lambda passed to assertThrows, and finally ASSERT by checking the
thrown TypeError and message; specifically update tests referencing scan,
operatorFn, and assertThrows so they explicitly separate setup (creating
operatorFn or args), execution (the () => scan(...) or () => operatorFn(...)
call) and the assertion (assertThrows(..., TypeError, "...")) into distinct
Arrange/Act/Assert sections.
🪄 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: 862af53e-a150-48e8-9ec4-1a07655495b2

📥 Commits

Reviewing files that changed from the base of the PR and between 2c9c0cb and 76618d1.

📒 Files selected for processing (20)
  • empty/mod.test.ts
  • finalize/mod.test.ts
  • from/mod.test.ts
  • interval/mod.test.ts
  • last-value-from/mod.test.ts
  • pairwise/deno.json
  • pairwise/mod.ts
  • reduce/deno.json
  • reduce/mod.test.ts
  • reduce/mod.ts
  • scan/deno.json
  • scan/mod.test.ts
  • scan/mod.ts
  • share/deno.json
  • share/mod.test.ts
  • share/mod.ts
  • switch-map/README.md
  • switch-map/deno.json
  • switch-map/mod.ts
  • tap/mod.test.ts

Comment thread last-value-from/mod.test.ts Outdated
Comment thread reduce/mod.test.ts
Comment thread scan/mod.test.ts
@alexanderharding alexanderharding merged commit dd2f167 into main May 21, 2026
2 checks passed
@alexanderharding alexanderharding deleted the cleanup branch May 21, 2026 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant