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 doesn't work well with custom compiler #1003

Closed
6 tasks done
wight554 opened this issue Mar 22, 2022 · 15 comments · Fixed by #1784
Closed
6 tasks done

vitest doesn't work well with custom compiler #1003

wight554 opened this issue Mar 22, 2022 · 15 comments · Fixed by #1784

Comments

@wight554
Copy link

Describe the bug

When using @rollup/plugin-typescript as compiler for tests I'm getting close timed out after 1000ms warning after successful run

Reproduction

wight554/blog-template#30
CI checks can be used for reference

System Info

System:
    OS: macOS 12.3
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
    Memory: 2.62 GB / 32.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.14.0 - ~/.nvm/versions/node/v16.14.0/bin/node
    Yarn: 3.1.1 - ~/.nvm/versions/node/v16.14.0/bin/yarn
    npm: 8.3.1 - ~/.nvm/versions/node/v16.14.0/bin/npm
    Watchman: 2021.10.04.00 - /usr/local/bin/watchman
  Browsers:
    Chrome: 99.0.4844.83
    Edge: 99.0.1150.46
    Firefox: 98.0.1
    Safari: 15.4
  npmPackages:
    vite: ^2.8.6 => 2.8.6 
    vitest: ^0.7.4 => 0.7.4

Used Package Manager

yarn

Validations

@wight554
Copy link
Author

I've tried adding some debug logging, and I see no reason why this happens
that's what @rollup/plugin-typescript shows:

entered exit
entered close
closingPromise undefined
closingPromise results [
  { status: 'fulfilled', value: undefined },
  { status: 'fulfilled', value: undefined }
]
closed successfully undefined
close timed out after 1000ms

And this is what swc compiler shows:

entered exit
entered close
closingPromise undefined
closingPromise results [
  { status: 'fulfilled', value: undefined },
  { status: 'fulfilled', value: undefined }
]
closed successfully undefined

My lib changes:

  async close() {
    console.log('entered close')
    var _a;
    console.log('closingPromise', this.closingPromise)
    if (!this.closingPromise) {
      this.closingPromise = Promise.allSettled([
        (_a = this.pool) == null ? void 0 : _a.close(),
        this.server.close()
      ].filter(Boolean)).then((results) => {
        console.log('closingPromise results', results)
        results.filter((r) => r.status === "rejected").forEach((err) => {
          this.error("error during close", err.reason);
        });
      });
    }
    return this.closingPromise;
  }
  async exit(force = false) {
    console.log('entered exit')
    setTimeout(() => {
      console.warn(`close timed out after ${CLOSE_TIMEOUT}ms`);
      process.exit();
    }, CLOSE_TIMEOUT).unref();
    var closed = await this.close();
    console.error('closed successfully', closed);
    if (force)
      process.exit();
  }

@wight554
Copy link
Author

wight554 commented Mar 24, 2022

Added https://www.npmjs.com/package/why-is-node-running
Got this:
There are 1196 handle(s) keeping the process running
Lot of stat watchers running, like this:

# STATWATCHER
node:internal/async_hooks:201                                                                         
node:internal/fs/watchers:105                                                                         
/Users/Volodymyr_Zhdanov/*/vite/blog-template/node_modules/typescript/lib/typescript.js:7022   - _fs.watchFile(fileName, { persistent: true, interval: pollingInterval }, fileChanged);
/Users/Volodymyr_Zhdanov/*/vite/blog-template/node_modules/typescript/lib/typescript.js:6272   - watcher: watchFile(fileName, function (fileName, eventKind) { return ts.forEach(callbacksCache.get(path), function (cb) { return cb(fileName, eventKind); }); }, pollingInterval, options),
/Users/Volodymyr_Zhdanov/*/vite/blog-template/node_modules/typescript/lib/typescript.js:6599   - return pollingWatchFile(fileName, callback, PollingInterval.Low, /*options*/ undefined);
/Users/Volodymyr_Zhdanov/*/vite/blog-template/node_modules/typescript/lib/typescript.js:113824 - watchFile: function (file, callback, pollingInterval, options) { return host.watchFile(file, callback, pollingInterval, options); },
/Users/Volodymyr_Zhdanov/*/vite/blog-template/node_modules/typescript/lib/typescript.js:113850 - factory[key].call(/*thisArgs*/ undefined, file, cb, flags, options, detailInfo1, detailInfo2) :
/Users/Volodymyr_Zhdanov/*/vite/blog-template/node_modules/typescript/lib/typescript.js:121887 - return watchFile(file, function (fileName, eventKind) { return callback(fileName, eventKind, path); }, pollingInterval, options, watchType);

@wight554
Copy link
Author

This doesn't happen with threads: false but this option makes random unexpected errors occur and breaks coverage reporting completely

@wight554 wight554 changed the title Getting close timed out warning when @rollup/plugin-typescript used as compiler vitest doesn't work well with custom compiler Mar 24, 2022
@wight554
Copy link
Author

wight554 commented Mar 24, 2022

Seems like it's due to how plugin handles exit, it depends on watchMode
https://github.com/rollup/plugins/blob/master/packages/typescript/src/index.ts#L87
Which leads to https://github.com/vitejs/vite/blob/main/packages/vite/src/node/server/pluginContainer.ts#L159
The problem is vitest is always running in vite serve mode, which doesn't sound correct for non watch mode

@wight554
Copy link
Author

cc @sheremet-va @patak-dev

@wight554
Copy link
Author

I've created small wrapper package https://www.npmjs.com/package/vite-plugin-typescript for this issue
But I'd really like to know if there's a better workaround

@sheremet-va
Copy link
Member

Not sure what we can do here? Maybe exit gracefully? Maybe an option? All plugins are run with watch: true, no?

@sheremet-va
Copy link
Member

Also I think this.meta is shared? So you don't actually need to wrap it? Just add a plugin, that sets watch to false?

@wight554
Copy link
Author

wight554 commented Apr 2, 2022

Not sure what we can do here? Maybe exit gracefully? Maybe an option? All plugins are run with watch: true, no?

I wonder why we should run them in watch: true when using non watch test mode
I've tried making watch: true for watch mode only but I can't make plugin read proper mode depending on env variable
I think it might make sense to do this from vitest

@sheremet-va
Copy link
Member

Not sure what we can do here? Maybe exit gracefully? Maybe an option? All plugins are run with watch: true, no?

I wonder why we should run them in watch: true when using non watch test mode I've tried making watch: true for watch mode only but I can't make plugin read proper mode depending on env variable I think it might make sense to do this from vitest

I think it is needed for some internal Vite process. It also makes sense because we are running a server and transform files when needed. So from our perspective everything is correct. If watch mode is disabled, how can Vite know not to end process?

@wight554
Copy link
Author

wight554 commented Apr 2, 2022

Not sure what we can do here? Maybe exit gracefully? Maybe an option? All plugins are run with watch: true, no?

I wonder why we should run them in watch: true when using non watch test mode I've tried making watch: true for watch mode only but I can't make plugin read proper mode depending on env variable I think it might make sense to do this from vitest

I think it is needed for some internal Vite process. It also makes sense because we are running a server and transform files when needed. So from our perspective everything is correct. If watch mode is disabled, how can Vite know not to end process?

Well then it might be good idea to expose VITEST_MODE earlier, so plugins can check it
And actually document it

@sheremet-va
Copy link
Member

Well then it might be good idea to expose VITEST_MODE earlier, so plugins can check it And actually document it

Mode is resolved after server has started, because it can be overwritten in your config. If we could resolve config before, it would be nice, but createServer does it inside a function (but even then plugins would already be initialized).

@wight554
Copy link
Author

wight554 commented Jul 26, 2022

Well then it might be good idea to expose VITEST_MODE earlier, so plugins can check it And actually document it

Mode is resolved after server has started, because it can be overwritten in your config. If we could resolve config before, it would be nice, but createServer does it inside a function (but even then plugins would already be initialized).

Is it still valid that vitest cannot signal it's running in watch mode to vite/rollup plugin api (e.g via config)?
Seen a lot of changes were made for vite3, wanna clarify if I still need workaround

@sheremet-va
Copy link
Member

Is it still valid that vitest cannot signal it's running in watch mode to vite/rollup plugin api (e.g via config)? Seen a lot of changes were made for vite3, wanna clarify if I still need workaround

No, Vitest watch mode has nothing to do with Vite. It's enabled by --watch or vitest run or inside config:

{
  test: { watch: true }
}

The last one is the reason why we can't set VITEST_MODE earlier.

@wight554
Copy link
Author

Is it still valid that vitest cannot signal it's running in watch mode to vite/rollup plugin api (e.g via config)? Seen a lot of changes were made for vite3, wanna clarify if I still need workaround

No, Vitest watch mode has nothing to do with Vite. It's enabled by --watch or vitest run or inside config:

{
  test: { watch: true }
}

The last one is the reason why we can't set VITEST_MODE earlier.

Also seen discussions about plugins option under test section.
Can potential plugins option make plugins more vitest aware or it would be a shortcut to simple process.env.VITEST check?

@github-actions github-actions bot locked and limited conversation to collaborators Jun 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants