Skip to content

feat(@schematics/angular): add option to migrate fakeAsync to Vitest fake timers#33111

Open
yjaaidi wants to merge 5 commits intoangular:mainfrom
yjaaidi:feat/add-refactor-fake-async-migration
Open

feat(@schematics/angular): add option to migrate fakeAsync to Vitest fake timers#33111
yjaaidi wants to merge 5 commits intoangular:mainfrom
yjaaidi:feat/add-refactor-fake-async-migration

Conversation

@yjaaidi
Copy link
Copy Markdown
Contributor

@yjaaidi yjaaidi commented Apr 30, 2026

PR Checklist

Please check to confirm your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

There is no migration from fakeAsync to Vitest fake timer APIs.

Issue Number: N/A

What is the new behavior?

The current migration transforms the following test:

describe('suite', () => {
  it("should work", fakeAsync(() => {
    let value = 0;

    setTimeout(() => value = 42, 100);

    tick(100);
    flushMicrotasks();
    flush();

    expect(value).toBe(42);
  });
);

into:

import { beforeAll, afterAll, vi } from 'vitest';


describe('suite', () => {

  beforeAll(() => {
    vi.useFakeTimers({ advanceTimeDelta: 1, shouldAdvanceTime: true });
  });
  afterAll(() => {
    vi.useRealTimers();
  });

  it("should work", fakeAsync(() => {
    let value = 0;

    setTimeout(() => value = 42, 100);

    await vi.advanceTimersByTimeAsync(100);
    await vi.advanceTimersByTimeAsync(0);
    await vi.runAllTimersAsync();

    expect(value).toBe(42);
  });
});

Does this PR introduce a breaking change?

  • Yes
  • No

Open Questions

  • This migration could be part of the jasmine-vitest migration. Should I moved it there and add an opt-out?
    Moved to jasmine-vitest migration under fakeAsync option which is false by default

  • flushMicrotasks can only be partially reproduced (i.e. explicit queueMicrotask calls), transforming it have high chances of producing a broken test. Should we just skip transforming tests that use it?
    Migrated to await vi.advanceTimersByTimeAsync(0);.

  • Should we generate code that measures time "manually" when the return value of flush is used?
    Generates a TODO.

  • Should we generate an "ad-hoc" function in the test file that reproduces flush's maxTurns option?
    Generates a TODO.

  • What should we do about processNewMacroTasksSynchronously option?
    Ignored.

  • This only migrates flush and tick if used inside fakeAsync but it's probably better to just replace in the whole file as users might create some wrappers. What do you think?
    Replaced everywhere.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds support for migrating Angular fakeAsync tests to Vitest by introducing a new fakeAsync option and corresponding transformers for tick, flush, and flushMicrotasks. The implementation converts these utilities to Vitest's vi fake timer API. Reviewers suggested scoping the fake timer hooks more narrowly to avoid side effects on sibling tests, disabling shouldAdvanceTime to better match fakeAsync semantics, and refining the flush transformer's logic for detecting discarded return values.

@yjaaidi yjaaidi force-pushed the feat/add-refactor-fake-async-migration branch from e90eef2 to fbd8e15 Compare April 30, 2026 14:57
@yjaaidi yjaaidi marked this pull request as draft April 30, 2026 15:01
@yjaaidi yjaaidi force-pushed the feat/add-refactor-fake-async-migration branch 2 times, most recently from cc415cf to 24627cd Compare April 30, 2026 15:21
Comment thread packages/schematics/angular/refactor/jasmine-vitest/schema.json
* @param methodeName The name of the vitest method to call.
* @returns The created identifier node.
*/
export function createViCallExpression(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I allowed my self to move this from ast-helpers to refactor-helpers so that it automatically adds pending vitest imports.

`,
},
{
description: 'should not replace `fakeAsync` if not used within a describe block',
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This exact scenario is not likely within Jasmine but fakeAsync could be used in some test factory or something similar.
In that case, we just bail.

// > beforeAll(() => {
// > vi.useFakeTimers({
// > advanceTimeDelta: 1,
// > shouldAdvanceTime: true
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Probably the best alternative to use fake timers without freezing other tests in the suite which rely on rely timers.

@yjaaidi yjaaidi marked this pull request as ready for review April 30, 2026 15:34
@yjaaidi yjaaidi changed the title feat(@schematics/angular): migrate fake async to Vitest fake timers feat(@schematics/angular): add option to migrate fakeAsync to Vitest fake timers Apr 30, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the fakeAsync transformation to the jasmine-vitest schematic, enabling the migration of Angular fakeAsync tests to Vitest's fake timers. It adds specific transformers for flush, flushMicrotasks, tick, and the fakeAsync wrapper, supported by new utility functions for AST manipulation. Review feedback highlights several areas for improvement: ensuring tick() arguments are preserved even when they are not numeric literals, maintaining parameters and type parameters when converting callbacks to async functions, adjusting Vitest timer settings to better match standard fakeAsync behavior, and correcting misleading comments regarding vi.useRealTimers().

Comment on lines +35 to +42
const durationNumericLiteral =
node.arguments.length > 0 && ts.isNumericLiteral(node.arguments[0])
? node.arguments[0]
: ts.factory.createNumericLiteral(0);

return ts.factory.createAwaitExpression(
createViCallExpression(ctx, 'advanceTimersByTimeAsync', [durationNumericLiteral]),
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The current implementation only preserves the duration if it is a numeric literal. If a variable or expression is passed to tick(), it is incorrectly transformed to 0. The expression should be preserved regardless of its type to ensure the test logic remains correct.

  const duration =
    node.arguments.length > 0 ? node.arguments[0] : ts.factory.createNumericLiteral(0);

  return ts.factory.createAwaitExpression(
    createViCallExpression(ctx, 'advanceTimersByTimeAsync', [duration]),
  );

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks legitimate. Can you address?

Copy link
Copy Markdown
Contributor Author

@yjaaidi yjaaidi May 1, 2026

Choose a reason for hiding this comment

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

Nice catch! Fixed by 2a00899

Comment on lines +110 to +117
return ts.factory.createArrowFunction(
[ts.factory.createModifier(ts.SyntaxKind.AsyncKeyword)],
undefined,
[],
undefined,
ts.factory.createToken(ts.SyntaxKind.EqualsGreaterThanToken),
ts.factory.createBlock(callbackBody.statements),
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

When transforming the fakeAsync callback to an async function, any existing parameters or type parameters should be preserved. Although fakeAsync callbacks typically don't have parameters, preserving them ensures compatibility with any custom wrappers or edge cases where types are explicitly defined.

Suggested change
return ts.factory.createArrowFunction(
[ts.factory.createModifier(ts.SyntaxKind.AsyncKeyword)],
undefined,
[],
undefined,
ts.factory.createToken(ts.SyntaxKind.EqualsGreaterThanToken),
ts.factory.createBlock(callbackBody.statements),
);
return ts.factory.createArrowFunction(
[ts.factory.createModifier(ts.SyntaxKind.AsyncKeyword)],
fakeAsyncCallback.typeParameters,
fakeAsyncCallback.parameters,
fakeAsyncCallback.type,
ts.factory.createToken(ts.SyntaxKind.EqualsGreaterThanToken),
ts.factory.createBlock(callbackBody.statements),
);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed by 3707db7

But not keeping the return type as it would be invalid given the function is turned into an async function. Transforming the return type to a Promise would be over-engineering as I can't imagine a scenario where such typing would be useful.

@yjaaidi yjaaidi force-pushed the feat/add-refactor-fake-async-migration branch from 20329da to e3ff0a2 Compare May 1, 2026 19:42
@yjaaidi yjaaidi force-pushed the feat/add-refactor-fake-async-migration branch from 874cbe9 to 1ed014b Compare May 1, 2026 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants