-
Notifications
You must be signed in to change notification settings - Fork 27
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
SuperJson Jest SWC run issue #184
Comments
same using vitest |
You might try to use a |
I deleted a comment about a different bug, the |
tried it but at least with vitest the problem is still there. I've also tried to install in the project the correct superjson version but same error:
|
issue with vitest is that dataTransport.js for example is compiled in this way:
vitest doesn't work with require so problem is not superjson (maybe also it, but not the only one) but how it is bundled the package |
It's a bit of a weird situation - we're compiling the package in a way that works well for Next.js, because that's our "target consumer". If your test environment now errors, I'm inclined to say "your test setup needs to be in line with your Next.js bundling setup, because otherwise your tests probably test completely different code that you actually bundle and run in production". But at the same time, I also feel your pain. In the short-to-medium term (talking days to weeks probably), I'm going to abstract away the core of this package to also work with other frameworks like Redwood, and at that time I'll have to revisit the bundling and make sure it works in more environments. In the very short term, I'm sorry I don't have a better solution. One last thing:
I'm a bit confused how vitest can even use React then, which heavily uses |
@phryneas thank you for the reply. |
@phryneas It is indeed a weir case, to clarify my situation, I have an Nrwl Nx Monorepo with Next.js configured, but I don't have this in the main app, I put it into a NextJS Library, which I think doesn't repro the NextJS build setup 1 to 1, for example, the jest configs and the TS configs already differ, the biggest diff I can spot is the transform, Next uses NextJS files
// Next jest.confi.ts
/* eslint-disable */
export default {
displayName: 'graph-budget',
preset: '../../jest.preset.js',
transform: {
'^(?!.*\\.(js|jsx|ts|tsx|css|json)$)': '@nx/react/plugins/jest',
'^.+\\.[tj]sx?$': ['babel-jest', { presets: ['@nx/next/babel'] }],
},
moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx'],
coverageDirectory: '../../coverage/apps/graph-budget',
}; // Next tsconfig.spec.json
{
"extends": "./tsconfig.json",
"compilerOptions": {
"outDir": "../../dist/out-tsc",
"module": "commonjs",
"types": ["jest", "node"],
"jsx": "react"
},
"include": [
"jest.config.ts",
"src/**/*.test.ts",
"src/**/*.spec.ts",
"src/**/*.test.tsx",
"src/**/*.spec.tsx",
"src/**/*.test.js",
"src/**/*.spec.js",
"src/**/*.test.jsx",
"src/**/*.spec.jsx",
"src/**/*.d.ts"
]
} NextJS Library files
// Library jest.confi.ts
/* eslint-disable */
export default {
displayName: 'neo4j-apollo',
preset: '../../jest.preset.js',
transform: {
'^.+\\.[tj]sx?$': [
'@swc/jest',
{
jsc: {
parser: { syntax: 'typescript', tsx: true },
transform: { react: { runtime: 'automatic' } },
},
},
],
},
moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx'],
coverageDirectory: '../../coverage/libs/neo4j-apollo',
}; // Library tsconfig.spec.json
{
"extends": "./tsconfig.json",
"compilerOptions": {
"outDir": "../../dist/out-tsc",
"module": "commonjs",
"types": ["jest", "node"]
},
"include": [
"jest.config.ts",
"src/**/*.test.ts",
"src/**/*.spec.ts",
"src/**/*.test.tsx",
"src/**/*.spec.tsx",
"src/**/*.test.js",
"src/**/*.spec.js",
"src/**/*.test.jsx",
"src/**/*.spec.jsx",
"src/**/*.d.ts"
]
} |
To avoid confusion for people with my latest comment, this is the latest workaround, not quite a definitive solution though. |
To keep you updated, I'm reworking the bundling (among other things) over in #189. There will be CommonJS and ESM for this package, depending on what your environment needs. There is just one problem arising: So far, we didn't do that, but we had checks like So, assuming you were using something like JSDOM until now, which "fakes" the global window object, your tests were always running the "browser version" of the hooks. With this new approach, I believe your tests will be running the "SSR version" of the hooks, unless you manually run your tests with I'm not 100% sure if that will work out nicely. Could you all please try out the PR release over in that other issue and give feedback if you get that running like you'd expect it to work over there? |
Wow, those are a lot of file changes, seems like you even moved stuff out of the global window, I'm not confident in my React & Next knowledge to understand all that, but I'll try looking into it on the weekend, as well as trying it out in my project. |
@phryneas I installed the PR commit, removed the npm overrides ( Nevermind, forgot this issue was with tests, with the PR installed I'm still having this issue, though now there's no mention of SuperJson: |
On the other hand, I went and installed the latest Maybe we can close this issue given the latest version works ok without workarounds... |
Issue
I tried recently in a new project to use this library again, not sure why but this time I can't run the tests, from the error message I can only infer it's the SuperJson library that has a non-commonjs dist
index.js
, have yet to find how to configure stuff so it includes that specific library in the compilation of TS.To be clear I do have other components & tests with imports, it's this one specifically that presents this error.
The text was updated successfully, but these errors were encountered: