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

css-post plugins crashes when using transformRequest without first starting a server #3798

Closed
6 tasks done
Pajn opened this issue Jun 14, 2021 · 9 comments
Closed
6 tasks done

Comments

@Pajn
Copy link

Pajn commented Jun 14, 2021

Describe the bug

While using the JS API, transformRequest can be used to get transformed versions of js/ts files without starting a server, but when doing the same on css files the css-post plugin crashes with:

❯ node server.js
(node:269183) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'get' of undefined
    at TransformContext.transform (/home/rasmus/Development/vite-repors/vite-transformRequest-css-crash-repro/node_modules/vite/dist/node/chunks/dep-bc228bbb.js:23139:56)
    at Object.transform (/home/rasmus/Development/vite-repors/vite-transformRequest-css-crash-repro/node_modules/vite/dist/node/chunks/dep-bc228bbb.js:44765:53)
    at async transformRequest (/home/rasmus/Development/vite-repors/vite-transformRequest-css-crash-repro/node_modules/vite/dist/node/chunks/dep-bc228bbb.js:59029:29)
    at async main (/home/rasmus/Development/vite-repors/vite-transformRequest-css-crash-repro/server.js:7:18)
(Use `node --trace-warnings ...` to show where the warning was created)
(node:269183) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:269183) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Reproduction

https://github.com/Pajn/vite-transformRequest-css-crash-repro

System Info

Output of npx envinfo --system --npmPackages vite,@vitejs/plugin-vue --binaries --browsers:

 npx envinfo --system --npmPackages vite,@vitejs/plugin-vue --binaries --browsers
npx: installed 1 in 0.727s

  System:
    OS: Linux 5.11 Ubuntu 21.04 (Hirsute Hippo)
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
    Memory: 8.59 GB / 30.99 GB
    Container: Yes
    Shell: 5.8 - /usr/bin/zsh
  Browsers:
    Chrome: 91.0.4472.101
    Firefox: 89.0
  npmPackages:
    vite: ^2.3.7 => 2.3.7

Used package manager: yarn

Logs

See Repro


Before submitting the issue, please make sure you do the following

@dejour
Copy link
Contributor

dejour commented Jun 14, 2021

not sure we should directly use transformRequest, if so, we can do null check on this line

cssModulesCache.get(config).get(id)

@Pajn
Copy link
Author

Pajn commented Jun 14, 2021

It's documented here https://vitejs.dev/guide/api-javascript.html so I would expect it to be supported

@patak-dev
Copy link
Member

@Pajn where is it documented that you can use transformRequest without starting the server? The issue you got is because there is a cache that is initialized at buildStart. I think it is expected that to use the server, it should be first initialized. Could you give us more context of why do you need to transform something before that?

@Pajn
Copy link
Author

Pajn commented Jun 15, 2021

@patak-js Not explicitly without starting the server, but it says "Programmatically resolve, load and transform a URL and get the result without going through the http request pipeline.". It seems weird to start a server and consume a port that you'll never use. It also works great with js and ts files, as long as no css files are imported making it more seem like a bug in the css plugin instead of a limitation of the API.

EDIT: Missed your final question

Could you give us more context of why do you need to transform something before that?

I'm trying to make an integration with @web/test-runner that works as good as @snowpack/web-test-runner-plugin. test-runner uses @web/dev-server and it's crucial for more advanced use cases that requires @web/test-runner-commands so I'm calling transformRequest in a handler from it, Vites dev server is and will never be used.
See the issue in material-svelte/vite-web-test-runner-plugin#11 for more context. I have that issue solved locally with adapting @snowpack/web-test-runner-plugin using transformRequest instead of, like vite-web-test-runner-plugin, starting both dev servers and doing redirects between them.

@patak-dev
Copy link
Member

@patak-js Not explicitly without starting the server, but it says "Programmatically resolve, load and transform a URL and get the result without going through the http request pipeline.". It seems weird to start a server and consume a port that you'll never use. It also works great with js and ts files, as long as no css files are imported making it more seem like a bug in the css plugin instead of a limitation of the API.

The issue is that the rollup plugins are intended to be used in the context of the server running. As with rollup, plugins uses buildStart to initialize the caches or resources they need.

EDIT: Missed your final question

Could you give us more context of why do you need to transform something before that?

I'm trying to make an integration with @web/test-runner that works as good as @snowpack/web-test-runner-plugin. test-runner uses @web/dev-server and it's crucial for more advanced use cases that requires @web/test-runner-commands so I'm calling transformRequest in a handler from it, Vites dev server is and will never be used.
See the issue in betaboon/vite-web-test-runner-plugin#11 for more context. I have that issue solved locally with adapting @snowpack/web-test-runner-plugin using transformRequest instead of, like vite-web-test-runner-plugin, starting both dev servers and doing redirects between them.

This looks like a valid use case though. I think it would be complex to introduce a feature to avoid the server connection. In your case, with respect to this comment material-svelte/vite-web-test-runner-plugin#11 (comment), couldn't you create a plugin for Vite that would take care of not transforming that file?

@Pajn
Copy link
Author

Pajn commented Jun 16, 2021

The issue is that the rollup plugins are intended to be used in the context of the server running. As with rollup, plugins uses buildStart to initialize the caches or resources they need.

Maybe there should be a way to start a build without also starting the server?

couldn't you create a plugin for Vite that would take care of not transforming that file?

I was planning to propose resolve.external, that would set build.rollupOptions.external, optimizeDeps.exclude and also require this patch:

diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts
index 5ef64be1..274dbc27 100644
--- a/packages/vite/src/node/config.ts
+++ b/packages/vite/src/node/config.ts
@@ -111,7 +111,7 @@ export interface UserConfig {
   /**
    * Configure resolver
    */
-  resolve?: ResolveOptions & { alias?: AliasOptions }
+  resolve?: ResolveOptions & { alias?: AliasOptions, external?: string[] }
   /**
    * CSS related options (preprocessors and CSS modules)
    */
diff --git a/packages/vite/src/node/plugins/importAnalysis.ts b/packages/vite/src/node/plugins/importAnalysis.ts
index a1ad68c4..d2a63acf 100644
--- a/packages/vite/src/node/plugins/importAnalysis.ts
+++ b/packages/vite/src/node/plugins/importAnalysis.ts
@@ -176,6 +176,10 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
           url = url.replace(base, '/')
         }
 
+        if (config.resolve.external?.includes(url)) {
+          return [url, url]
+        }
+
         const resolved = await this.resolve(url, importer)
 
         if (!resolved) {

This would to my understanding make it equal to packageOptions.external in snowpack. With that and my custom test runner plugin I have @web/test-runner-visual-regression working.

@patak-dev
Copy link
Member

Maybe there should be a way to start a build without also starting the server?

Re-checking our discussion, this is already the case in middleware mode. You can init your server using and it will properly call buildStart and optimizeDeps, so you can use transformRequest after that

  const server = await vite.createServer({
    clearScreen: false,
    server: {
      middlewareMode: 'ssr'
    }
  })

@brion-fuller
Copy link

The issue is that the rollup plugins are intended to be used in the context of the server running. As with rollup, plugins uses buildStart to initialize the caches or resources they need.

Maybe there should be a way to start a build without also starting the server?

couldn't you create a plugin for Vite that would take care of not transforming that file?

I was planning to propose resolve.external, that would set build.rollupOptions.external, optimizeDeps.exclude and also require this patch:

diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts
index 5ef64be1..274dbc27 100644
--- a/packages/vite/src/node/config.ts
+++ b/packages/vite/src/node/config.ts
@@ -111,7 +111,7 @@ export interface UserConfig {
   /**
    * Configure resolver
    */
-  resolve?: ResolveOptions & { alias?: AliasOptions }
+  resolve?: ResolveOptions & { alias?: AliasOptions, external?: string[] }
   /**
    * CSS related options (preprocessors and CSS modules)
    */
diff --git a/packages/vite/src/node/plugins/importAnalysis.ts b/packages/vite/src/node/plugins/importAnalysis.ts
index a1ad68c4..d2a63acf 100644
--- a/packages/vite/src/node/plugins/importAnalysis.ts
+++ b/packages/vite/src/node/plugins/importAnalysis.ts
@@ -176,6 +176,10 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
           url = url.replace(base, '/')
         }
 
+        if (config.resolve.external?.includes(url)) {
+          return [url, url]
+        }
+
         const resolved = await this.resolve(url, importer)
 
         if (!resolved) {

This would to my understanding make it equal to packageOptions.external in snowpack. With that and my custom test runner plugin I have @web/test-runner-visual-regression working.

Do you have an example of this you could share? I am currently running into the same issue.

@antfu
Copy link
Member

antfu commented Sep 9, 2021

A workaround:

Instead for server.listen, use pluginContainer.buildStart to initialize the plugins

const server = await vite.createServer(serverConfig)
await server.pluginContainer.buildStart({})

Refer:

@antfu antfu closed this as completed Sep 9, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Sep 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants