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

Remove proload for config loading #5778

Merged
merged 6 commits into from
Jan 6, 2023
Merged

Remove proload for config loading #5778

merged 6 commits into from
Jan 6, 2023

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Jan 6, 2023

Changes

Close #5748
Fix #5459

Note: 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 with createFs since loadConfigFromFile 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.

@changeset-bot
Copy link

changeset-bot bot commented Jan 6, 2023

🦋 Changeset detected

Latest 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

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Jan 6, 2023
Comment on lines -210 to -212
// Reset NODE_ENV to initial value
// Vite's `createServer()` sets NODE_ENV to 'development'!
process.env.NODE_ENV = nodeEnv;
Copy link
Member Author

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)) {
Copy link
Member Author

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.

@matthewp matthewp merged commit 49ab4f2 into main Jan 6, 2023
@matthewp matthewp deleted the remove-proload branch January 6, 2023 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove Proload for loading config Issue with astro add and @astrojs/mdx
2 participants