Skip to content

test(signals): repair type checks and async regression coverage - #3781

Merged
ryansolid merged 3 commits into
solidjs:nextfrom
everton-dgn:fix/signals-test-types
Oct 5, 2026
Merged

ryansolid merged 3 commits into
solidjs:nextfrom
everton-dgn:fix/signals-test-types

Conversation

@everton-dgn

Copy link
Copy Markdown

Summary

The test-inclusive Signals TypeScript check reports 78 diagnostics on next. Several tests also stopped observing the behavior their names describe: action tests read the removed activeTransition property, a stream error handler is passed in the wrong argument, and the effect GC test constructs a WeakRef from the imperative phase's null owner.

This updates 31 test files without changing production code, configuration or dependencies. The commits separate fixture types and signatures, action completion ordering, and error/GC observations. Deliberately invalid calls remain invalid at runtime, and optional fixture keys remain absent.

How did you test this change?

  • node node_modules/typescript/bin/tsc --noEmit -p packages/signals/tsconfig.json --pretty false: zero diagnostics, down from 78 on the base.
  • All affected tests with GC enabled: 421 passed, 2 expected failures, 2 skipped.
  • pnpm --filter @solidjs/signals exec vitest run tests/gc.test.ts --pool forks --execArgv=--expose-gc --configLoader runner --no-cache: 7 passed.
  • Full Signals suite with --maxWorkers 4: 4,892 passed, 2 expected failures, 2 skipped across 256 files.
  • A mutation that runs the held flush before the injected operation fails all four action-window regressions.
  • After tightening the error identity assertion, restoring mocks on timeout and correcting the guide example, all 126 selected tests and the complete TypeScript check passed again.
  • Prettier and git diff --check passed.

An initial full-suite run hit a timing-sensitive refetch failure. That unchanged test passed on both the base and patched checkout in isolation, and the final complete run passed with four workers. Its assertions and timeouts were left unchanged.

Preserve refreshable accessor types, instantiate deferred gates, and fix
imports and call signatures used by the existing regression tests. Keep
deliberately invalid callbacks at an explicit runtime-test boundary and
keep optional fixture properties absent at runtime.

No production, configuration or dependency changes.
The L2 scheduler no longer exposes activeTransition. Reading that missing
property made the old condition pass without observing the intended
completion window.

Await the action while holding scheduler microtasks, inject the
operation, then release the flush. Assert the ordering and restore the
mock even when an action stalls until the test timeout.

All four regressions reject a mutation that flushes before the operation.
Pass the error handler in the effect bundle and assert the received
error while retaining the memo's throwing read. Capture the effect owner
in the compute phase so the GC regression observes a valid weak target.

The seven explicit-GC tests pass. With the related fixture and action
test repairs, the test-inclusive TypeScript check has zero diagnostics
and the full Signals suite passes 4,892 tests.
@changeset-bot

changeset-bot Bot commented Oct 4, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 3434fd6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@codspeed

codspeed Bot commented Oct 4, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 185 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing everton-dgn:fix/signals-test-types (3434fd6) with next (1b9ceb6)

Open in CodSpeed

Footnotes

  1. 3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@ryansolid
ryansolid merged commit b4dd868 into solidjs:next Oct 5, 2026
7 checks passed
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.

2 participants