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

Package imported directly and through dependency results in different instances, breaking equality checks #6795

Closed
6 tasks done
joaof9 opened this issue Oct 25, 2024 · 2 comments

Comments

@joaof9
Copy link

joaof9 commented Oct 25, 2024

Describe the bug

First issue so apologies if I missed something

I'm trying to migrate from jest to vite and ran across this

import { takeLatest, take, call, put } from 'redux-saga/effects';
import { testSaga } from 'redux-saga-test-plan';

function identity() {
  return 'value';
}

function* mainSaga() {
  const action = yield takeLatest('a', identity);
}

describe('saga', () => {
  it('works with unit tests', () => {
    testSaga(mainSaga, 40, 2).next().takeLatest('a', identity).next().isDone();
  });
});

I get

Expected
--------
{
  '@@redux-saga/IO': true,
  combinator: false,
  type: 'FORK',
  payload: {
    context: null,
    fn: [Function: takeLatest], <--- this is the part that fails equality check
    args: [ 'a', [Function: identity] ]
  }
}

Actual
------
{
  '@@redux-saga/IO': true,
  combinator: false,
  type: 'FORK',
  payload: {
    context: null,
    fn: [Function: takeLatest], <--- this is the part that fails equality check
    args: [ 'a', [Function: identity] ]
  }
}

This fails because the takeLatest used and imported here, is not the same as the one imported by the 'redux-saga-test-plan' package when validating the takeLatest() part of the test. However the same test works correctly in jest.

It's also easily noticeable when mocking the dependency ourselves,

vi.mock(import('redux-saga/effects'), async (importOriginal) => {
    const actual = await importOriginal();

    return {
        ...actual,
        takeLatest: () => 'some thing',
    };
});

where we get

Expected
--------
{
  '@@redux-saga/IO': true,
  combinator: false,
  type: 'FORK',
  payload: {
    context: null,
    fn: [Function: takeLatest],
    args: [ 'a', [Function: identity] ]
  }
}

Actual
------
'some thing'

which could maybe be a workaround but it seems vite can't mock dependencies of dependencies correctly? Again, note that mocking this with jest would result in both instances being mocked and the test passing.

Anyway the gist of it seems to be that when some package is imported directly in a test file and from another dependency, it results in two different instances of it, and trying to do some equality check between those two fails.

I have tried adding these packages to inline dependencies, optimizeDeps and other options I've seen around, but nothing seems to work

Reproduction

https://stackblitz.com/edit/vitest-dev-vitest-fxzhfs?file=test/suite.test.ts

System Info

N/A

Used Package Manager

npm

Validations

@hi-ogawa
Copy link
Contributor

This fails because the takeLatest used and imported here, is not the same as the one imported by the 'redux-saga-test-plan' package when validating the takeLatest() part of the test.

This is because Vitest picks up redux-saga/effects's module export https://publint.dev/[email protected], but redux-saga-test-plan is cjs only https://publint.dev/[email protected] so it will transitively picks up redux-saga/effects's cjs export.

One workaround is to force using redux-saga/effects's cjs export on Vitest, which might be possible using resolve.alias https://stackblitz.com/edit/vitest-dev-vitest-r9knwr?file=vite.config.ts

You can search for "dual package" to find a similar issue https://github.com/vitest-dev/vitest/issues?q=is:issue "dual package"

It's also easily noticeable when mocking the dependency ourselves, ... which could maybe be a workaround but it seems vite can't mock dependencies of dependencies correctly?

vi.mock doesn't work for transitive import of cjs, so the workaround I mentioned above doesn't work either. Probably optimizer.ssr.enabled: true is needed there, but it doesn't look working either, so there might be an issue with vi.mock and optimized deps.

@joaof9
Copy link
Author

joaof9 commented Oct 28, 2024

One workaround is to force using redux-saga/effects's cjs export on Vitest, which might be possible using resolve.alias https://stackblitz.com/edit/vitest-dev-vitest-r9knwr?file=vite.config.ts

Gotcha, that makes sense and it works perfectly, thank you

vi.mock doesn't work for transitive import of cjs, so the workaround I mentioned above doesn't work either. Probably optimizer.ssr.enabled: true is needed there, but it doesn't look working either, so there might be an issue with vi.mock and optimized deps.

Yeah, about this part, this definitely feels like it should work someway, it's not hard to imagine a scenario where one would need to mock a transitive import of cjs. But not really the focus for this issue.

Thanks again

@joaof9 joaof9 closed this as completed Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants