-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Remove proload for config loading #5778
Conversation
🦋 Changeset detectedLatest commit: 8b0a4ba The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
// Reset NODE_ENV to initial value | ||
// Vite's `createServer()` sets NODE_ENV to 'development'! | ||
process.env.NODE_ENV = nodeEnv; |
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.
This removal undoes #5700 (review) since we're already using Vite 4. Thought I'll make this change since this PR is config related.
].map((p) => path.join(root, p)); | ||
|
||
for (const file of paths) { | ||
if (fsMod.existsSync(file)) { |
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.
The previous function uses stat
to check file existence, I switched to existsSync
since it should be the same.
Changes
Close #5748
Fix #5459
resolveConfigPath
to only search for a config path, and not load the config itself, this fixes Issue withastro add
and@astrojs/mdx
#5459Note: I tried to use Vite's
loadConfigFromFile
instead, but it doesn't bring much benefit for us, and prevents named exports from the Astro config if we ever need it. It's also harder to test withcreateFs
sinceloadConfigFromFile
always read from the actual filesystem.Testing
Existing config loading test should work. Since this removes a proload feature, I don't think there's new test to add.
Docs
n/a. Ideally loading the config with Vite should always work, so this change shouldn't need a docs update.