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

[Bug report]: build with vite mode parallel with cause error #1250

Closed
nonzzz opened this issue Feb 3, 2023 · 17 comments
Closed

[Bug report]: build with vite mode parallel with cause error #1250

nonzzz opened this issue Feb 3, 2023 · 17 comments
Assignees
Labels
bug Something isn't working need review

Comments

@nonzzz
Copy link

nonzzz commented Feb 3, 2023

Description

Background

Maybe it's look likes a bug. Refer nonzzz/vite-plugin-compression#18

I try to change vuepress it's vite build logic ( from parallel to serial. It works well

Form rollup/rollup#3506 i think we shouldn't parallel run vite build :)

Reproduction

https://github.com/u2sb/www.u2sb.com/tree/vite-compression-plugin

Used Package Manager

pnpm

System Info

$ vuepress build docs --clean-cache --clean-temp
✔ Cleaning temp - done in 20ms
✔ Cleaning cache - done in 2ms
✔ Initializing and preparing data - done in 3.45s
⠹ Compiling with vite
(!) Some chunks are larger than 1024 kBs after minification. Consider:
- Using dynamic import() to code-split the application
- Use build.rollupOptions.output.manualChunks to improve chunking: https://rollupjs.org/guide/en/#outputmanualchunks
- Adjust chunk size limit for this warning via build.chunkSizeWarningLimit.
✖ Compiling with vite - failed in 74.95s
Error: ENOENT: no such file or directory, open 'D:\githubDownLoad\www.u2sb.com\docs\.vuepress\.temp\.server\assets\app-77c74ba0.js'
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
@nonzzz nonzzz changed the title [Bug report] [Bug report]: build with vite mode parallel with cause error Feb 3, 2023
@Mister-Hope
Copy link
Member

Mister-Hope commented Feb 3, 2023

I do think you need to provide detailed inforamation about what our problem is, at least the issue is now thrown at your plugin.

You may need to explain clearly:

  1. What's the central action of the issue part
  2. What we did to let this happen (our fault or just compatibility breaking
  3. Any possible solution

@nonzzz
Copy link
Author

nonzzz commented Feb 3, 2023

@Mister-Hope Thanks for you feedback. I will update error message

@nonzzz
Copy link
Author

nonzzz commented Feb 3, 2023

Sorry for the waitting. From the error info. I judged that the error occurred in the generatorBundle stage. Becasue currently. vuepress internal build logic is parallel. But it will casue rollup bundle context loose right hash assets file

@nonzzz
Copy link
Author

nonzzz commented Feb 3, 2023

According

[clientOutput, serverOutput] = await Promise.all([
 viteBuild(clientConfig),
  viteBuild(serverConfig)
]);

Will emit an error

But with serial

image

@Mister-Hope

This comment was marked as off-topic.

@Mister-Hope Mister-Hope added the bug Something isn't working label Feb 7, 2023
@Mister-Hope
Copy link
Member

I got what the issue is.

Here we are generating 2 config:

https://github.com/vuepress/vuepress-next/blob/fd2e241235fc87c66ce59ba22c8e27521c594fbd/packages/bundler-vite/src/resolveViteConfig.ts

But we are sharing same plugin instance across client/server in build process. @meteorlxy

@nonzzz
Copy link
Author

nonzzz commented Feb 7, 2023

Yes. You are right.It will cause concurrent.

@nonzzz
Copy link
Author

nonzzz commented Feb 7, 2023

I think we should generator two difference app instance? Or build config?

@nonzzz
Copy link
Author

nonzzz commented Feb 7, 2023

parallel build won't cause error ( My mistake

@Mister-Hope
Copy link
Member

Mister-Hope commented Feb 7, 2023

I am not familiar with vite, but if you do think that 2 plugin instance is required to fix this, then a breaking change must be made at api, we shall use a callable function for bundler:

-   bundler: viteBundler({
-     // vite bundler config
-   })
+   bundler: () => viteBundler({
+     // vite bundler config
+   })

@nonzzz
Copy link
Author

nonzzz commented Feb 7, 2023

Sounds bad. That will be a break change : (

@nonzzz
Copy link
Author

nonzzz commented Feb 8, 2023

I look vuepress source code. I noticed https://github.com/vuepress/vuepress-next/blob/fd2e241235fc87c66ce59ba22c8e27521c594fbd/packages/bundler-vite/src/plugins/mainPlugin.ts#L93-L114

Currently, Us build config across client/server in build process. But will change the process of rollup generating hash. See rollup

By the way. rollup generateBundle is sequential : )

I think there two way to sovle.

  • Remove Promise.all ( Using serial.
  • Isolate two config list.

@Mister-Hope
Copy link
Member

Remove Promise.all ( Using serial.

This is definitely not our consideration, our build time is already long enough, and our primary goal is to improve speed

@meteorlxy
Copy link
Member

Closing as this has been stale for a long time. Vite version and VuePress version has updated a lot.

Feel free to open a new issue with the latest version & context if it's still a problem

@Mister-Hope
Copy link
Member

Mister-Hope commented Jun 20, 2024

I got what the issue is.

Here we are generating 2 config:

https://github.com/vuepress/vuepress-next/blob/fd2e241235fc87c66ce59ba22c8e27521c594fbd/packages/bundler-vite/src/resolveViteConfig.ts

But we are sharing same plugin instance across client/server in build process. @meteorlxy

I don't think this is should be resolved. A lot of plugins can not work in multiple instance.

Running successfully in multiple instance Means plugin cannot have any local state outside any hooks, But as plugins are designed callable, they should be allowed to have such things.

@Mister-Hope
Copy link
Member

I am not familiar with vite, but if you do think that 2 plugin instance is required to fix this, then a breaking change must be made at api, we shall use a callable function for bundler:

-   bundler: viteBundler({
-     // vite bundler config
-   })
+   bundler: () => viteBundler({
+     // vite bundler config
+   })

This should be a valid fix, but this is a breaking change. @meteorlxy

@Mister-Hope
Copy link
Member

Suggest remaining it open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working need review
Projects
None yet
Development

No branches or pull requests

3 participants