Skip to content

Cleanup#99

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

Cleanup#99
alexanderharding merged 10 commits into
mainfrom
cleanup

Conversation

@alexanderharding

@alexanderharding alexanderharding commented May 21, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • Bug Fixes

    • Fixed abort signal timing to prevent emitting values after an observable is aborted.
    • Corrected documentation references to build automation workflows across all packages.
  • Documentation

    • Clarified immutability rules for exported object instances in TypeScript examples.
    • Updated behavior descriptions and examples for better accuracy.
  • Chores

    • Bumped package versions across all modules.
    • Updated test expectations and added constructor validation tests.

Review Change Stack

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

coderabbitai Bot commented May 21, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR applies a coordinated monorepo release cycle: normalizing all README workflow path references from Windows to POSIX style, bumping versions across all package manifests, clarifying root documentation types, fixing core library test naming, applying targeted behavioral improvements to for-await-of and for-in modules, correcting code example syntax, and refactoring ReplaySubject constructor logic with validation tests.

Changes

Observable Monorepo Release

Layer / File(s) Summary
README workflow path normalization
all/README.md, async-await/README.md, at/README.md, audit/readme.md, behavior-subject/README.md, broadcast-subject/README.md, catch-error/README.md, core/README.md, debounce/README.md, defer/README.md, delay/README.md, distinct-until-changed/README.md, distinct/README.md, drop/README.md, empty/README.md, exhaust-map/README.md, expand/README.md, fetch/README.md, filter/README.md, finalize/README.md, flat-map/README.md, flat/README.md, for-await-of/README.md, for-in/README.md, for-of/README.md, from/README.md, interval/README.md, keep-alive/README.md, last-value-from/README.md, map/README.md, materialize/README.md, merge-map/README.md, merge/README.md, never/README.md, of/README.md, pairwise/README.md, pipe/README.md, race/README.md, reduce/README.md, repeat/README.md, replay-subject/README.md, rxjs-interop/README.md, scan/README.md, share/README.md, switch-map/README.md, take/README.md, tap/README.md, throttle/README.md, throw-error/README.md, timeout/README.md, until/README.md
All README files updated to reference .github/workflows/publish.yml using forward slashes instead of the old Windows-style backslash path (.github\workflows\publish.yml).
Package version releases
all/deno.json, async-await/deno.json, at/deno.json, audit/deno.json, behavior-subject/deno.json, broadcast-subject/deno.json, catch-error/deno.json, core/deno.json, debounce/deno.json, defer/deno.json, delay/deno.json, distinct-until-changed/deno.json, distinct/deno.json, drop/deno.json, empty/deno.json, exhaust-map/deno.json, expand/deno.json, fetch/deno.json, filter/deno.json, finalize/deno.json, flat-map/deno.json, flat/deno.json, for-await-of/deno.json, for-in/deno.json, for-of/deno.json, from/deno.json, interval/deno.json, keep-alive/deno.json, last-value-from/deno.json, map/deno.json, materialize/deno.json, merge-map/deno.json, merge/deno.json, never/deno.json, of/deno.json, pairwise/deno.json, pipe/deno.json, race/deno.json, reduce/deno.json, repeat/deno.json, replay-subject/deno.json, rxjs-interop/deno.json, scan/deno.json, share/deno.json, switch-map/deno.json, take/deno.json, tap/deno.json, throttle/deno.json, throw-error/deno.json, timeout/deno.json, until/deno.json
All package manifests receive coordinated minor version bumps across the monorepo (e.g., 0.20.0 → 0.21.0, 0.6.0 → 0.7.0).
Root README type and immutability clarification
README.md
TypeScript example for ExampleConstructor is corrected to use the concrete type Example instead of placeholder A. Immutability requirement is clarified to apply only to public/exported object instances, constructors, and prototypes.
Core library test cleanup and naming fixes
core/observable.test.ts, core/observer.test.ts, core/subject.test.ts
Minor test updates: trailing semicolon removed from a comment in the resubscribe test, variable renamed in the observer return-throw error test, and test description updated to reflect that Subject constructor ignores extra arguments.
for-await-of abort signal check ordering
for-await-of/mod.ts
Abort signal check (observer.signal.aborted) is repositioned to execute before observer.next(value), preventing emissions of values processed after an abort signal is observed.
for-in inherited enumerable keys support
for-in/README.md, for-in/deno.json, for-in/mod.test.ts
Documentation is updated to state that forIn(object) emits both own and inherited enumerable string keys (unlike Object.keys). A new test validates that inherited keys are emitted in the correct order after own keys.
Documentation code example syntax corrections
delay/README.md, delay/mod.ts, distinct-until-changed/README.md, distinct-until-changed/mod.ts, distinct/README.md
Extra closing parenthesis is removed from the delay(NaN) code example in both README and JSDoc, and the distinct-until-changed custom comparator example is reformatted to use multiline pipe and subscribe calls for improved readability.
ReplaySubject constructor refactoring and validation tests
replay-subject/mod.ts, replay-subject/deno.json, replay-subject/mod.test.ts
Constructor logic is rewritten from an inline ternary into explicit step-by-step statements. Two new test cases verify that the constructor throws TypeError when called with zero arguments or a non-numeric count parameter.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 The monorepo hops to version skies,
Paths straightened from backslash lies,
Tests refined and behaviors aligned,
Each package bumped, together designed.
Forward slashes guide the way,
Release day hops out to play!

🚥 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 doesn't convey meaningful information about the changeset. Consider using a more specific title that describes the main categories of changes, such as 'Fix workflow paths and bump package versions' or 'Correct workflow paths across all packages and update versions'.
✅ 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.

🧹 Nitpick comments (1)
replay-subject/mod.test.ts (1)

449-463: ⚡ Quick win

Use explicit Arrange/Act/Assert sections in the two new constructor tests.

These tests are correct functionally, but they should follow the required AAA structure for consistency with the repo’s test standard.

♻️ Suggested update
 Deno.test("ReplaySubject.constructor should throw when called with no arguments", () => {
+  // Arrange
+  const constructWithoutArgs = () =>
+    new ReplaySubject(...([] as unknown as ConstructorParameters<typeof ReplaySubject>));
+
+  // Act / Assert
   assertThrows(
-    () => new ReplaySubject(...([] as unknown as ConstructorParameters<typeof ReplaySubject>)),
+    constructWithoutArgs,
     TypeError,
     "1 argument required but 0 present",
   );
 });
 
 Deno.test("ReplaySubject.constructor should throw when count is not a number", () => {
+  // Arrange
+  const constructWithInvalidCount = () => new ReplaySubject("3" as unknown as number);
+
+  // Act / Assert
   assertThrows(
-    () => new ReplaySubject("3" as unknown as number),
+    constructWithInvalidCount,
     TypeError,
     "Parameter 1 is not of type 'Number'",
   );
 });

As per coding guidelines **/*.test.ts: Use Arrange/Act/Assert pattern with Deno.test for test cases.

🤖 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 `@replay-subject/mod.test.ts` around lines 449 - 463, Split each of the two
constructor tests for ReplaySubject into explicit Arrange/Act/Assert sections:
for the "should throw when called with no arguments" test, Arrange any needed
setup (none), Act by invoking new ReplaySubject(...([] as unknown as
ConstructorParameters<typeof ReplaySubject>)) inside a closure, and Assert by
calling assertThrows with that closure, the expected TypeError and message;
similarly for the "should throw when count is not a number" test, arrange the
invalid argument ("3" as unknown as number), act by invoking new ReplaySubject
with it inside a closure, and assert with assertThrows expecting TypeError and
the "Parameter 1 is not of type 'Number'" message—keep the existing test names
and assertions but reorganize into clear Arrange / Act / Assert comment blocks
or logical sections within the Deno.test callbacks.
🤖 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.

Nitpick comments:
In `@replay-subject/mod.test.ts`:
- Around line 449-463: Split each of the two constructor tests for ReplaySubject
into explicit Arrange/Act/Assert sections: for the "should throw when called
with no arguments" test, Arrange any needed setup (none), Act by invoking new
ReplaySubject(...([] as unknown as ConstructorParameters<typeof ReplaySubject>))
inside a closure, and Assert by calling assertThrows with that closure, the
expected TypeError and message; similarly for the "should throw when count is
not a number" test, arrange the invalid argument ("3" as unknown as number), act
by invoking new ReplaySubject with it inside a closure, and assert with
assertThrows expecting TypeError and the "Parameter 1 is not of type 'Number'"
message—keep the existing test names and assertions but reorganize into clear
Arrange / Act / Assert comment blocks or logical sections within the Deno.test
callbacks.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f4bdba2b-b5ea-433a-af8b-f329af4a185a

📥 Commits

Reviewing files that changed from the base of the PR and between da4cd2a and c54158f.

📒 Files selected for processing (112)
  • README.md
  • all/README.md
  • all/deno.json
  • async-await/README.md
  • async-await/deno.json
  • at/README.md
  • at/deno.json
  • audit/deno.json
  • audit/readme.md
  • behavior-subject/README.md
  • behavior-subject/deno.json
  • broadcast-subject/README.md
  • broadcast-subject/deno.json
  • catch-error/README.md
  • catch-error/deno.json
  • core/README.md
  • core/deno.json
  • core/observable.test.ts
  • core/observer.test.ts
  • core/subject.test.ts
  • debounce/README.md
  • debounce/deno.json
  • defer/README.md
  • defer/deno.json
  • delay/README.md
  • delay/deno.json
  • delay/mod.ts
  • distinct-until-changed/README.md
  • distinct-until-changed/deno.json
  • distinct-until-changed/mod.ts
  • distinct/README.md
  • distinct/deno.json
  • drop/README.md
  • drop/deno.json
  • empty/README.md
  • empty/deno.json
  • exhaust-map/README.md
  • exhaust-map/deno.json
  • expand/README.md
  • expand/deno.json
  • fetch/README.md
  • fetch/deno.json
  • filter/README.md
  • filter/deno.json
  • finalize/README.md
  • finalize/deno.json
  • flat-map/README.md
  • flat-map/deno.json
  • flat/README.md
  • flat/deno.json
  • for-await-of/README.md
  • for-await-of/deno.json
  • for-await-of/mod.ts
  • for-in/README.md
  • for-in/deno.json
  • for-in/mod.test.ts
  • for-of/README.md
  • for-of/deno.json
  • from/README.md
  • from/deno.json
  • interval/README.md
  • interval/deno.json
  • keep-alive/README.md
  • keep-alive/deno.json
  • last-value-from/README.md
  • last-value-from/deno.json
  • map/README.md
  • map/deno.json
  • materialize/README.md
  • materialize/deno.json
  • merge-map/README.md
  • merge-map/deno.json
  • merge/README.md
  • merge/deno.json
  • never/README.md
  • never/deno.json
  • of/README.md
  • of/deno.json
  • pairwise/README.md
  • pairwise/deno.json
  • pipe/README.md
  • pipe/deno.json
  • race/README.md
  • race/deno.json
  • reduce/README.md
  • reduce/deno.json
  • repeat/README.md
  • repeat/deno.json
  • replay-subject/README.md
  • replay-subject/deno.json
  • replay-subject/mod.test.ts
  • replay-subject/mod.ts
  • rxjs-interop/README.md
  • rxjs-interop/deno.json
  • scan/README.md
  • scan/deno.json
  • share/README.md
  • share/deno.json
  • switch-map/README.md
  • switch-map/deno.json
  • take/README.md
  • take/deno.json
  • tap/README.md
  • tap/deno.json
  • throttle/README.md
  • throttle/deno.json
  • throw-error/README.md
  • throw-error/deno.json
  • timeout/README.md
  • timeout/deno.json
  • until/README.md
  • until/deno.json

@alexanderharding alexanderharding merged commit 2c9c0cb into main May 21, 2026
2 checks passed
@alexanderharding alexanderharding deleted the cleanup branch May 21, 2026 05:35
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