vitest errors on cyclic imports

everett1992 commented 1 year ago

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

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
[X] The provided reproduction is a minimal reproducible example of the bug.

stackblitz[bot] commented 1 year ago

Start a new pull request in StackBlitz Codeflow.

everett1992 commented 1 year ago

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 commented 1 year ago

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 }

lysekjan commented 1 year ago

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 commented 1 year ago

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()