search

vitest-dev / vitest

Next generation testing framework powered by Vite.
https://vitest.dev
MIT License
12.88k stars 1.15k forks source link

Vitest seems resolved duplicate copies of same package and broke react context singleton #4180

Closed smeng9 closed 1 week ago

smeng9 commented 1 year ago

Describe the bug

It seems when a project has nested dependency DepndencyB through DependencyA and the project also directly uses DependencyB, a duplicate copy of DependencyB would be created when tested through vitest.

I have checked the lock file and made sure there are no duplicate copies installed. Also vite dev server and production build seems is not affected by this issue.

I have tried multiple different vite/vitest config settings to bypass the issue and no luck yet. I have tried changing optimizeDeps.exclude, resolve.dedupe from vite, and server.deps, deps.optimizer from vitest and still can't get test to pass.

Reproduction

https://stackblitz.com/edit/vitejs-vite-bkzhc2?file=src%2Fposts.test.tsx

You can run the test with yarn install && yarn vitest. I expect all 4 of them to pass, but only PostCreate and TextInput passed, and PostSpyCreate and TextSpyInput failed.

In the above example, the test project depends on react-admin which depends on react-hook-form. The test project also uses some contexts and hooks from react-hook-form.

In my TextSpyInput I used a hook to get form value, this hook only works within the form context. SimpleForm in PostCreate provides the FormProvider context and the page can render without error using yarn dev

In vitest it seems throwing error, suggesting the context is not resolved correctly by vitest. As a control group, I have used FormDataConsumer which internally uses the same hook in the test, and it does not throw any error.

System Info

❯ npx envinfo --system --npmPackages '{vitest,@vitest/*,vite,@vitejs/*}' --binaries --browsers
Need to install the following packages:
  envinfo@7.10.0
Ok to proceed? (y) y

  System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 16.20.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 9.4.2 - /usr/local/bin/npm
    pnpm: 8.6.10 - /usr/local/bin/pnpm

Used Package Manager

yarn

Validations

stackblitz[bot] commented 1 year ago

Fix this issue in StackBlitz Codeflow Start a new pull request in StackBlitz Codeflow.

smeng9 commented 1 year ago

After more research, the issue seems can be solved by setting resolve.mainFields to ["module"]

smeng9 commented 1 year ago

Linked with the merge request on react-admin https://github.com/marmelab/react-admin/pull/9309. Closing this for now

smeng9 commented 1 year ago

Reopening because setting resolve.mainFields to ["module"] breaks some other packages.

I hope when resolving package that has both esm and cjs format, vitest can have the same behavior as vite. Vite does not need special handling for resolve.mainFields

Dunqing commented 1 year ago

You can try setting resolve.mainFields to ['module', 'jsnext:main', 'jsnext'] (default value for mainFields)

smeng9 commented 1 year ago

Hi Dunqing, I have tried to set the list to the default value of vite for mainFields as you suggested but doing so breaks some other tests for the reason in vitest’s comment about mainFields.

I have checked why vitest set this to empty list in its comments, because some packages have cjs format in the module field. Vite seems is ok with that, vitest is unhappy about that.

Is it possible to set mainFields to use 'module' just for a single package? E.g. let mainFields support a dictionary with package name as key and field name as value? I really don’t want to use the last resort to manually patch the package.json in node_modules.

Get Outlook for iOShttps://aka.ms/o0ukef

From: Dunqing @.> Sent: Wednesday, September 27, 2023 1:06:16 AM To: vitest-dev/vitest @.> Cc: Meng, Shaoyu @.>; State change @.> Subject: Re: [vitest-dev/vitest] Vitest seems resolved duplicate copies of same package and broke react context singleton (Issue #4180)

You can try setting resolve.mainFields to ['module', 'jsnext:main', 'jsnext'] (default value for mainFields)

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/vitest-dev/vitest/issues/4180*issuecomment-1736758274__;Iw!!DZ3fjg!6E01voaDxwy_mxKHSYjp9XeAzFUxTNXG-E2IJTpPntTg3Tsd1oOIP4Wpj44RCyG1Spvo8Od815_b93VoaszTHgF5mlc$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AJHAEC3U4PL757KKUYQVZ6LX4O65RANCNFSM6AAAAAA5HE4HME__;!!DZ3fjg!6E01voaDxwy_mxKHSYjp9XeAzFUxTNXG-E2IJTpPntTg3Tsd1oOIP4Wpj44RCyG1Spvo8Od815_b93VoaszTlAYtCVk$. You are receiving this because you modified the open/close state.Message ID: @.***>

Dunqing commented 1 year ago

Perhaps you could use alias to map react-admin to the corresponding esm file

smeng9 commented 1 year ago

I have tried your suggestion on the test case https://stackblitz.com/edit/vitejs-vite-bkzhc2?file=src%2Fposts.test.tsx

I have two options: either use alias for react-admin to resolve an esm copy, or use alias to force react-hook-form resolve to a cjs copy

I tried setting the alias field to "react-admin": "react-admin/dist/esm/index.js", it reports the same error

Then I tried react-hook-form, since it only has top level exports for "." and "./package.json", I cannot use alias just to grab an internal copy of the cjs file.

Dunqing commented 1 year ago

Then I tried react-hook-form, since it only has top level exports for "." and "./package.json", I cannot use alias just to grab an internal copy of the cjs file.

You should use an absolute path. You can see https://stackblitz.com/edit/vitejs-vite-i7kgbn?file=vite.config.js

smeng9 commented 1 year ago

Thanks a lot! You have saved my day.

Finally used this workaround to remove duplicates and get react-hook-form resolved the same copy of react context.