Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Insufficient documentation for migration to 0.31 #3332

Closed
6 tasks done
golergka opened this issue May 9, 2023 · 9 comments
Closed
6 tasks done

Insufficient documentation for migration to 0.31 #3332

golergka opened this issue May 9, 2023 · 9 comments

Comments

@golergka
Copy link

golergka commented May 9, 2023

Describe the bug

I have this mocking code in my codebase:

const mockedGoogleLogin = vi.fn();

vi.mock("@react-oauth/google", async () => {
  const Google = await vi.importActual<typeof import("@react-oauth/google")>(
    "@react-oauth/google"
  );
  return {
    ...Google,
    useGoogleLogin: vi.fn(() => mockedGoogleLogin),
  };
});

And this check later on:

    expect(mockedGoogleLogin).toHaveBeenCalledOnce();

In 0.30 version, this check passed, but in 0.31, it does not. I think it's connected to vi.hoisted change.

Now, I don't think it's a bug in vitest code. However, I think that there's insufficient documentation on the way to properly migrate — so this bug is about documentation quality, not code.

I don't understand how exactly did behaviour of the above code change. I would assume that if mockedGoogleLogin didn't exist at hoist time, it should have thrown something trying to access it. And I don't understand exact steps needed to rewrite it for 0.31 — some examples and migration guide would be helpful.

Reproduction

  1. Decrease IQ to my level
  2. Try to port working code to 0.31 using only release notes
  3. Get confused

System Info

I'm not really smart

Used Package Manager

npm

Validations

@github-actions
Copy link

github-actions bot commented May 9, 2023

Hello @golergka. Please provide a minimal reproduction using a GitHub repository or StackBlitz. Issues marked with need reproduction will be closed if they have no activity within 3 days.

@golergka
Copy link
Author

golergka commented May 9, 2023

@sheremet-va this is an issue about documentation, not code. The only steps needed to reproduce is are already explained in the issue:

  1. Decrease IQ to my level
  2. Try to port working code to 0.31 using only release notes
  3. Get confused

@sheremet-va
Copy link
Member

@sheremet-va this is an issue about documentation, not code. The only steps needed to reproduce is are already explained in the issue:

  1. Decrease IQ to my level
  2. Try to port working code to 0.31 using only release notes
  3. Get confused

I don't understand the problem that needs documenting. The changes to mocks API were not breaking, you don't need to migrate anything.

@donalmurtagh
Copy link

I don't understand the problem that needs documenting. The changes to mocks API were not breaking

@sheremet-va Then why is it described in the release notes as a breaking change?

In my project, some tests are failing after upgrading from v0.30.1 to v0.31.0, so it is a breaking change for me.

@sheremet-va
Copy link
Member

In my project, some tests are failing after upgrading from v0.30.1 to v0.31.0, so it is a breaking change for me.

It's breaking if Vitest is used with public API, the usage did not change. If you have any problems, create a reproduction and we will fix it.

@donalmurtagh
Copy link

It's breaking if Vitest is used with public API, the usage did not change.

Do you mean it's a change to the API because hoisted didn't exist in the previous version, but all of the functions that existed in the previous version should behave the same in v0.31.0?

@sheremet-va
Copy link
Member

Do you mean it's a change to the API because hoisted didn't exist in the previous version, but all of the functions that existed in the previous version should behave the same in v0.31.0?

By public API I mean startVitest from vitest/node entry point (although it's still quite hard to break). But the main reason why it's breaking is that you cannot use browser in poolMatchGlobs. Since it was removed in the same commit, it got marked together. I can move the mocking change out of the "breaking changes" section manually, I guess.

@golergka
Copy link
Author

golergka commented May 9, 2023

Thanks for clarifying. Yes, removing the "Breaking change" in changelogs would solve this issue — right now it looks like it needs migrating.

This means my issues are actually a bug with vitest code and not me being stupid. I'll create a separate issue for this.

@sheremet-va
Copy link
Member

I changed the release notes. I also suspect that you might've had the same issue described in #3340 If so, it should be fixed in the next version.

Please, feel free to create an issue with reproduction if you have any unexpected changes that were caused with the latest release.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants