Closed Aslemammad closed 1 year ago
The latest updates on your projects. Learn more about Vercel for Git ↗︎
Name | Status | Preview | Comments | Updated (UTC) |
---|---|---|---|---|
valtio | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | May 1, 2023 11:36pm |
This pull request is automatically built and testable in CodeSandbox.
To see build info of the built libraries, click here or the icon next to each commit SHA.
Latest deployment of this branch, based on commit 65ddb7c0b30a17e5ee646a1dad0070121f35172f:
Sandbox | Source |
---|---|
React | Configuration |
React Typescript | Configuration |
React Browserify | Configuration |
React Snowpack | Configuration |
React Parcel | Configuration |
The tests are passing, I do not know what is happening with types in the lower versions.
So far, it looks good to me.
@arjunvegda Would you like to take a look? (We don't merge this yet.)
Nice stuff @Aslemammad!
I'm running into the same exact issue on Jotai 😅 . The tests for TS versions 3.9.7
and 3.8.3
are failing.
It appears that those TS versions are ignoring the skipLibCheck
😟. I tried a few tricks like strict: false
, exclude: ['node_modules/**]
, and adding // @ts-ignore
but no luck.
I'll continue digging!
@arjunvegda Thank you so much, yea, I didn't find any solution neither.
@dai-shi I didn't have access to my main computer that's why I didn't type those functions :)
I didn't have access to my main computer
No need to rush. Take your time.
When you get a chance to access to your computer, please merge main
.
@arjunvegda Thank you so much, yea, I didn't find any solution neither.
I think I managed to make it work 😅. I did the following
@testing-library/user-event
and @testing-library/react
that are compatible with TS 3.9.7 and 3.9.8. See jotai/commits/b73e4310803315defebc124f90d0f9a9aaef4376vitest
as there is no way to have TS skip them (probably a bug in 3.9.x ?). See jotai/commits/b73e4310803315defebc124f90d0f9a9aaef4376vitest
within the types.d.ts
. See jotai/commits/0c628591c05476fc3820e737ca160b1bd719ca08parse5
module, as vitest relies on it. See jotai/commits/d0edaf6891450ad4a8eec0e69c74b358f930d724It's not great, but it does the job for now. Let me know what you think!
Nice job! Thank you so much!
@dai-shi Wdyt?
Nice job! Thank you so much!
@dai-shi Wdyt?
I just reviewed https://github.com/pmndrs/jotai/pull/1839. Seems like they are reasonable hacks.
@arjunvegda Could you have a closer look to find why 21db220 doesn't work? what's the difference from jotai and zustand...
Hmm? It looks like it's loading two instances. 🤔
Never mind. The last commit fixes the test, and it's probably because it somehow points to a different file.
Hmm? It looks like it's loading two instances. 🤔
Ah! This explains it. if I change the import in utils/watch.ts#L1 to valtio/vanilla
it works as expected 🤔
It appears that vite is resolving modules differently based on how we import the paths (relative vs absolute)?
- import { subscribe } from '../../vanilla.ts'
+ import { subscribe } from 'valtio/vanilla'
@arjunvegda Could you have a closer look to find why 21db220 doesn't work? what's the difference from jotai and zustand...
Jotai and Zustand do not appear to have conflicting imports within test files, so it is working fine. However, we could reproduce the issue in those libs too.
This will cause modules to be duplicated and the test will fail.
import { expect, it } from 'vitest'
import { getDefaultStore } from 'jotai/vanilla/store'
import { getDefaultStore as getDefaultStoreFromVanilla } from 'jotai/vanilla'
it('should match the default store', () => {
const storeA = getDefaultStoreFromVanilla()
const storeB = getDefaultStore()
expect(storeA).toBe(storeB)
})
This could be fixed by deduping imports using @rollup/plugin-node-resolve in vite.config.ts
though
@arjunvegda Thanks for investigating. Things are clear now. I think for now what we have is fine.
@arjunvegda Thanks for investigating. Things are clear now. I think for now what we have is fine.
This happens because relative paths in aliases are used. Vitest doesn't resolve them relative to root. Each time "valtio" is mentioned it will try to load it relative to the file it is imported from and fail. But Vitest fallbacks to the original replacement. This creates two different instances in the modules graph. The fix is to never use relative paths in aliases.
The code you have here will probably break in the next Vitest release where we fixed this.
The code you have here will probably break in the next Vitest release where we fixed this.
@sheremet-va Thank you for the explanation. We'll try removing it in the next release of Vitest. cc @dai-shi
So, always to use __dirname
? That's also clear (as it's consistent).
So, always to use
__dirname
? That's also clear (as it's consistent).
Yes. Or import.meta.url
.
Related Issues
643