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

perf: use fsp in more cases #12553

Merged
merged 2 commits into from Mar 25, 2023
Merged

perf: use fsp in more cases #12553

merged 2 commits into from Mar 25, 2023

Conversation

patak-dev
Copy link
Member

Description

Use fs/promises to avoid blocking the main thread in more cases. While we are scanning or pre-bundling, we are also processing for example.


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@stackblitz
Copy link

stackblitz bot commented Mar 23, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

cert: readFileIfExists(cert),
key: readFileIfExists(key),
pfx: readFileIfExists(pfx),
ca: await readFileIfExists(ca),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Promise.all ??

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, done at vitejs/vite@de5e611 (#12553). Also simplified the logic.

@@ -1099,7 +1100,7 @@ async function loadConfigFromBundledFile(
// for cjs, we can register a custom loader via `_require.extensions`
else {
const extension = path.extname(fileName)
const realFileName = fs.realpathSync(fileName)
const realFileName = await fsp.realpath(fileName)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: The fsp version of realpath uses .native by default, which would cause file read errors on Windows for network drives. But since we're not doing that here, this change should be safe.

Copy link
Contributor

@fwouts fwouts Apr 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This caused #12923, the async version returns a path with uppercase volume letter C:\... whereas the earlier version returns it lowercase, so the equality check on line 1107 no longer works.

@bluwy bluwy added performance Performance related enhancement p2-nice-to-have Not breaking anything but nice to have (priority) labels Mar 25, 2023
@patak-dev patak-dev merged commit e9b92f5 into main Mar 25, 2023
18 checks passed
@patak-dev patak-dev deleted the perf/use-fsp-in-more-cases branch March 25, 2023 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-nice-to-have Not breaking anything but nice to have (priority) performance Performance related enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants