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

Load dotenv/config before reading the config #3085

Merged
merged 3 commits into from
Jan 16, 2024
Merged

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Jan 15, 2024

This PR also fixes an issue when running Toolpad in --dev where we get fighting vite devservers messing with each other's optimized deps. Just forcing optimization when running in dev mode seems to fix it.

@Janpot Janpot added the core Infrastructure work going on behind the scenes label Jan 15, 2024
@Janpot Janpot changed the title Load dotenv asap on the serverworker Load dotenv/config before reading the config Jan 15, 2024
@Janpot Janpot marked this pull request as ready for review January 15, 2024 17:25
@Janpot Janpot requested a review from a team January 16, 2024 08:26
Copy link
Member

@apedroferreira apedroferreira left a comment

Choose a reason for hiding this comment

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

Looks good!

@@ -1,3 +1,4 @@
import 'dotenv/config';
Copy link
Member

Choose a reason for hiding this comment

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

I forgot that we also had the option to load the env in the CLI commands, just mentioning it in case it's better than importing like this in multiple files.

Copy link
Member Author

Choose a reason for hiding this comment

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

It was the only way I could make it work. But you are right, we should never build things that we don't understand. I've done some further root cause analysis and after inspecting the bundles I noticed some imports before dotenv/config. I immediately remembered an issue I'd seen before. I disabled code splitting and now the ordering is predictable. Adding a big fat comment to explain why the setting is there.

@Janpot Janpot merged commit 624571d into mui:master Jan 16, 2024
11 checks passed
@Janpot Janpot deleted the dotenv branch January 16, 2024 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants