fix(vite-node): externalize network imports by hi-ogawa · Pull Request #4987 · vitest-dev/vitest

hi-ogawa · 2024-01-17T02:52:59Z

Description

Closes Cannot import from external URLs #4949

As shown in the reproduction, currently network import is not externalized by vite-node and Vite's transformRequest ends up with a following error:

 FAIL  test/basic.test.ts [ test/basic.test.ts ]
Error: Failed to load url https://esm.sh/[email protected] (resolved id: https://esm.sh/[email protected]). Does the file exist?
 ❯ loadAndTransform node_modules/vite/dist/node/chunks/dep-V3BH7oO1.js:49777:21

Vite's ssrLoadModule didn't support network imports previously but it is changed to externalize it in vitejs/vite#15599, so Vite-node side change also aligns with Vite.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
Ideally, include a test that fails without this PR but passes with it.
Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Tests

Run the tests with pnpm test:ci.

Documentation

If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

netlify · 2024-01-17T02:53:06Z

✅ Deploy Preview for fastidious-cascaron-4ded94 ready!

Name	Link
🔨 Latest commit	`587b2bf`
🔍 Latest deploy log	https://app.netlify.com/sites/fastidious-cascaron-4ded94/deploys/65a7ab2dcc9df5000871f015
😎 Deploy Preview	https://deploy-preview-4987--fastidious-cascaron-4ded94.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

This reverts commit eb2f72a.

…k-imports' into fix-vite-node-externalize-network-imports

hi-ogawa · 2024-01-17T10:27:19Z

test/network-imports/vitest.config.ts

+      // not supported?
+      //   FAIL  test/basic.test.ts [ test/basic.test.ts ]
+      //   Error: ENOENT: no such file or directory, open 'http://localhost:9602/[email protected]'
+      //    ❯ Object.openSync node:fs:596:3
+      //    ❯ readFileSync node:fs:464:35
+      vmThreads: {
+        execArgv: ['--experimental-network-imports'],
+      },


It looks like it's not working on vmThreads. I'll investigate further to see if it's simply NodeJS limitation.

I think every import is delegated to our VM implementation, and it just doesn't support http.

Thanks for the hint! I haven't had a chance to look at how vmThreads works yet, so I'll look around a bit to get the idea.
I suppose it's still okay even if it doesn't work on vmThreads?

Yes, we can disable this test for vm threads/forks for now.

I think it's a separate feature request (maybe create an issue for it). Shouldn't be hard to implement.

sheremet-va · 2024-01-17T10:59:16Z

I am not sure how to contact you otherwise, but I sent an e-mail to the address listed in your GitHub profile. Please have a look when you have time 😄

fix(vite-node): externalize network imports

cfdb38d

hi-ogawa added 5 commits January 17, 2024 12:08

ci: tweak playwright

eb2f72a

test: add network-imports test

aa0d036

Revert "ci: tweak playwright"

3c99b8a

This reverts commit eb2f72a.

test: tweak

74cce1f

test: use local server instead of esm.sh

ab91ffc

hi-ogawa marked this pull request as ready for review January 17, 2024 04:07

hi-ogawa added 3 commits January 17, 2024 18:47

Merge branch 'main' into fix-vite-node-externalize-network-imports

3f934c8

test: test forks + skip vmThreads

e56382b

Merge remote-tracking branch 'origin/fix-vite-node-externalize-networ…

587b2bf

…k-imports' into fix-vite-node-externalize-network-imports

hi-ogawa commented Jan 17, 2024

View reviewed changes

sheremet-va approved these changes Jan 17, 2024

View reviewed changes

sheremet-va merged commit 21f5744 into vitest-dev:main Jan 17, 2024

hi-ogawa deleted the fix-vite-node-externalize-network-imports branch January 17, 2024 22:40

hi-ogawa mentioned this pull request Jan 18, 2024

network imports not working on vmThreads and vmForks pools #4995

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

fix(vite-node): externalize network imports#4987

fix(vite-node): externalize network imports#4987
sheremet-va merged 9 commits intovitest-dev:mainfrom
hi-ogawa:fix-vite-node-externalize-network-imports

hi-ogawa commented Jan 17, 2024 •

edited

Loading

Uh oh!

netlify bot commented Jan 17, 2024 •

edited

Loading

Uh oh!

hi-ogawa Jan 17, 2024

Uh oh!

sheremet-va Jan 17, 2024

Uh oh!

hi-ogawa Jan 17, 2024

Uh oh!

sheremet-va Jan 17, 2024 •

edited

Loading

Uh oh!

sheremet-va commented Jan 17, 2024

Uh oh!

Uh oh!

Comments

Conversation

hi-ogawa commented Jan 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

Tests

Documentation

Changesets

Uh oh!

netlify bot commented Jan 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for fastidious-cascaron-4ded94 ready!

Uh oh!

hi-ogawa Jan 17, 2024

Choose a reason for hiding this comment

Uh oh!

sheremet-va Jan 17, 2024

Choose a reason for hiding this comment

Uh oh!

hi-ogawa Jan 17, 2024

Choose a reason for hiding this comment

Uh oh!

sheremet-va Jan 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sheremet-va commented Jan 17, 2024

Uh oh!

hi-ogawa commented Jan 17, 2024 •

edited

Loading

netlify bot commented Jan 17, 2024 •

edited

Loading

sheremet-va Jan 17, 2024 •

edited

Loading