-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
fix: Vite cannot load configuration files in the link directory (#4180) #4181
Conversation
@Shinigami92 what about check this PR? <3 |
packages/vite/src/node/config.ts
Outdated
@@ -889,7 +889,7 @@ async function loadConfigFromBundledFile( | |||
const extension = path.extname(fileName) | |||
const defaultLoader = require.extensions[extension]! | |||
require.extensions[extension] = (module: NodeModule, filename: string) => { | |||
if (filename === fileName) { | |||
if (filename === fs.realpathSync(fileName)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@patak-js Wasn't there something about realpathSync
that it is bad? Cause it is slow or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know, we are not using it in the codebase. But we are using its async version realPath
here:
sourceRoot = await fs.realpath( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think fs.realpathSync(fileName)
will be called many times, if it is here and require
exists inside the config.
How about caching it like this?
const realFileName = fs.realpathSync(fileName)
require.extensions[extension] = (module: NodeModule, filename: string) => {
if (filename === realFileName) {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense @sapphi-red, would you like to send a commit with that change to this PR directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated 👍
Description
fix #4180
Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).