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

vitest errors on cyclic imports #4143

Open
6 tasks done
everett1992 opened this issue Sep 18, 2023 · 8 comments
Open
6 tasks done

vitest errors on cyclic imports #4143

everett1992 opened this issue Sep 18, 2023 · 8 comments
Labels
p2-edge-case Bug, but has workaround or limited in scope (priority) upstream vite-node

Comments

@everett1992
Copy link
Contributor

Describe the bug

When importing certain modules with cyclical imports vitest will evaluate a file before it's imports are undefined, causing unexpected TypeError: _ is undefined errors. I was not able to reproduce this error in esbuild, vite, or nodejs.

Reproduction

Unfortunately I haven't been able to create a minimal reproduction. The only example I have is importing ion-js. The vitest error is

TypeError: IonTypes is undefined
 ❯ file:/home/projects/vitest-dev-vitest-cpxaqv/node_modules/ion-js/dist/es6/es6/dom/Blob.js:5:1

I'm reporting this as a vitest error because I can't reproduce it in any other system.

vitest

https://stackblitz.com/edit/vitest-dev-vitest-cpxaqv?file=test%2Fbasic.test.ts

import { expect, it } from 'vitest';

it('imports ion-js', async () => {
  const { load } = await import('ion-js');
  expect(load("1").numberValue()).toEqual(1)
});

it('requires ion-js', async () => {
  const { load } = require('ion-js');
  expect(load("1").numberValue()).toEqual(1)
});

esbuild

echo 'import { load } from "ion-js"; console.log(load("1"))' | npx esbuild --analyze  --bundle --outfile="test.js"

test.js                                                          197.7kb  100.0%
 ├ node_modules/ion-js/dist/es6/es6/IonParserTextRaw.js           41.5kb   21.0%
...
test.js  197.7kb

⚡ Done in 23ms

node test.js
[Number (_Integer): 1] { _numberValue: 1, _bigIntValue: null }

node.js

node --input-type 'module' -e 'import { load } from "ion-js"; console.log(load("1"))'
[Number (Integer): 1] { _numberValue: 1, _bigIntValue: null }

This uses dist/commonjs/es6, not dist/es6/es6 like vitest and esbuild because ion-js doesn't build node compatible esm. I ran a codemod converting the package to true node esm (adds extensions and type:module) and got the same result.

vite

I tried to reproduce this in vite client and server side but never saw the error.

https://stackblitz.com/edit/vitejs-vite-mmhd5y?file=server.js

// server.js
import { createServer } from 'vite';

const vite = await createServer({
  server: {
    middlewareMode: true,
  },
  appType: 'custom',
});
await vite.ssrLoadModule('/entry-server.js');
// entry-server.js
import { load } from 'ion-js';
console.log(load('1'));
node server
[Number (Integer): 1] { _numberValue: 1, _bigIntValue: null }

https://github.com/amazon-ion/ion-js/blob/master/src/dom/Blob.ts#L1
https://github.com/amazon-ion/ion-js/blob/master/src/Ion.ts#L134
https://github.com/amazon-ion/ion-js/blob/master/src/IonTypes.ts#L16

System Info

(See stack blitz)
vitest 0.34.4

Used Package Manager

npm

Validations

@stackblitz
Copy link

stackblitz bot commented Sep 18, 2023

@everett1992
Copy link
Contributor Author

There error is here, IonTypes is undefined.
We can see IonTypes is imported from Ion.js

import { IonTypes, toBase64 } from "../Ion";
import { Lob } from "./Lob";
export class Blob extends Lob(IonTypes.BLOB) {

https://github.com/amazon-ion/ion-js/blob/master/src/dom/Blob.ts#L9

Ion.js re-exports IonTypes from IonTypes.js

export { IonTypes } from "./IonTypes";

https://github.com/amazon-ion/ion-js/blob/master/src/Ion.ts#L134

Which defines a keys for IonType instances, like an enum.

import { IonType } from "./IonType";

/** Enumeration of the Ion types. */
export const IonTypes = {
  NULL: new IonType(0, "null", true, false, false, false),

https://github.com/amazon-ion/ion-js/blob/master/src/IonTypes.ts#L16

@everett1992
Copy link
Contributor Author

everett1992 commented Sep 18, 2023

Okay, I have a minimal reproduction now with a few lines can be imported with node but fail in vitest.

// ion-js
// ├── dom
// │   ├── Blob.js
// │   └── index.js
// ├── Ion.js
// └── IonTypes.js

// ion-js/Ion.js
export { IonTypes } from "./IonTypes.js";
import * as dom from "./dom/index.js";
export { dom };

// ion-js/IonTypes.js
export const IonTypes = {
    BLOB: "Blob",
};

// ion-js/dom/index.js
export { Blob } from "./Blob.js";

// ion-js/dom/Blob.js
import { IonTypes } from "../Ion.js";
export const Blob =  IonTypes.BLOB

The issue is related to import * as dom from "./dom/index.js"; in Ion.js. If that is replaced with export * as dom from './dom/index.js' or import { Blob } from './dom/Blob.js' the issue doesn't reproduce.

stateDiagram-v2
    [*] --> Ion.js
    Ion.js --> IonTypes.js: export { IonTypes }
    Ion.js --> dom/index.js: import * as dom
    dom/index.js --> dom/Blob.js: export { Blob }
    dom/Blob.js --> Ion.js: import { IonTypes }

Loading

@lysekjan
Copy link

Same problem here.
I have reproduced that issue on a simple repo with minimal configuration:
https://github.com/lysekjan/vitest-imports

Just run npm run test and you get the exact error "{smt} is not a function"

Version

vitest 0.34.4

@sheremet-va
Copy link
Member

sheremet-va commented Sep 21, 2023

Looks like this is a Vite SSR limitation. This code also fails there (Vitest reuses the same transforms):

import { createServer } from 'vite';
const server = await createServer()
const { callFactory } = await server.ssrLoadModule('./src/module/utils.ts')
callFactory()
await server.close()

@JonasDoesThings
Copy link

Is there any workaround available? (besides refactoring the codebase) 😞

@sheremet-va sheremet-va added p2-edge-case Bug, but has workaround or limited in scope (priority) and removed bug labels Feb 16, 2024
@Dunqing Dunqing mentioned this issue Mar 23, 2024
6 tasks
@zangab
Copy link

zangab commented Jul 9, 2024

hey guys, any updates on this?

@AriPerkkio
Copy link
Member

There's an open issue on Vite for this: vitejs/vite#14048

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-edge-case Bug, but has workaround or limited in scope (priority) upstream vite-node
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants