Skip to content

fix(@schematics/angular): keep the default getter access type of spyOnProperty in refactor-jasmine-vitest - #34543

Merged
alan-agius4 merged 1 commit into
angular:mainfrom
thekhegay:jasmine-vitest-spyonproperty-get
Oct 28, 2026
Merged

alan-agius4 merged 1 commit into
angular:mainfrom
thekhegay:jasmine-vitest-spyonproperty-get

Conversation

@thekhegay

Copy link
Copy Markdown
Contributor

PR Checklist

PR Type

  • 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?

Issue Number: N/A

refactor-jasmine-vitest drops default access type of spyOnProperty
Jasmine uses 'get' when third argument is missing, vi.spyOn without it is typed and run as method spy

This spec passes on Jasmine:

class Session {
  get user(): string {
    return 'real';
  }
}

it('stubs the getter', () => {
  const session = new Session();
  spyOnProperty(session, 'user').and.returnValue('mocked');
  expect(session.user).toBe('mocked');
});

ng g @schematics/angular:refactor-jasmine-vitest on 22.3.0-next.3 turns it into vi.spyOn(session, 'user').mockReturnValue('mocked') and ng test fails to build:

✘ [ERROR] TS2769: No overload matches this call.

Without type check Vitest throws vi.spyOn() can only spy on a function at runtime

What is the new behavior?

transformPrimarySpy adds 'get' when spyOnProperty has two arguments, explict access type is kept as is
Same spec converts to vi.spyOn(session, 'user', 'get').mockReturnValue('mocked') and passes under ng test

Existing case in jasmine-spy_spec.ts expected vi.spyOn(service, 'myProp').mockReturnValue(undefined) for bare spyOnProperty, so it pinned this output
Updated it and added two cases, chained one and explicit 'set', updated and chained ones fail on main

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Bare spyOnProperty(obj, 'prop') still gets .mockReturnValue(undefined) from #33266, so after this change it compiles only when getter type allows undefined, same as bare spyOn on method that returns string

…nProperty in refactor-jasmine-vitest

Jasmine spies on getter when `spyOnProperty` gets no access type, migration turned `spyOnProperty(obj, 'prop')` into `vi.spyOn(obj, 'prop')`, which Vitest treats as method spy, so converted spec failed with TS2769
now `'get'` is passed when access type is omited, explicit one is kept

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request updates the Jasmine-to-Vitest schematic transformer to correctly handle spyOnProperty calls. When spyOnProperty is invoked with only two arguments, the transformer now appends 'get' as the third argument to vi.spyOn to match Jasmine's default behavior. Additionally, unit tests have been updated and added to verify this default behavior, chained calls, and explicit access types. There are no review comments, and we have no feedback to provide.

@clydin clydin added the target: patch This PR is targeted for the next patch release label Oct 27, 2026
@alan-agius4 alan-agius4 added the action: merge The PR is ready for merge by the caretaker label Oct 28, 2026
@alan-agius4
alan-agius4 merged commit 4ea7c67 into angular:main Oct 28, 2026
42 checks passed
@alan-agius4

Copy link
Copy Markdown
Collaborator

This PR was merged into the repository. The changes were merged into the following branches:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker area: @schematics/angular target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants