Refactor config loading #7622
Refactor config loading #7622
Conversation
|
github-actions
bot
added
the
pkg: astro
| @@ -102,13 +102,10 @@ async function handleConfigError( | |||
| error(logging, 'astro', `Unable to load ${path ? colors.bold(path) : 'your Astro config'}\n`); | |||
| if (e instanceof ZodError) { | |||
| console.error(formatConfigErrorMessage(e) + '\n'); | |||
| telemetry.record(eventConfigError({ cmd, err: e, isFatal: true })); | |||
There was a problem hiding this comment.
eventConfigError only accepts ZodError. This was added in #7316
| const tempConfigPath = path.join( | ||
| root, | ||
| `.temp.${Date.now()}.config${path.extname(configPath)}` | ||
| ); | ||
|
|
||
| const currentConfigContent = await fsMod.promises.readFile(configPath, 'utf-8'); | ||
| await fs.promises.writeFile(tempConfigPath, currentConfigContent); |
There was a problem hiding this comment.
This custom write flow is converted to
await import(pathToFileURL(configPath).toString() + '?t=' + Date.now());in vite-load.ts. This was initially added in #4578 for proload support which we now don't use.
| if (!configPath) return undefined; | ||
|
|
||
| // Create a vite server to load the config | ||
| return await loadConfigWithVite({ |
There was a problem hiding this comment.
Since we have the configPath guard here, we can refactor loadConfigWithVite to return the config directly, instead of { value, configPath }, because we already know the configPath.
| async function tryLoadConfig( | ||
| configOptions: LoadConfigOptions, | ||
| root: string | ||
| ): Promise<TryLoadConfigResult | undefined> { | ||
| ): Promise<Record<string, any> | undefined> { |
There was a problem hiding this comment.
While we're refactoring, I want to propose the following change, with two options:
- rename
tryLoadConfigtoloadConfigand always return aRecord<string, any>. The function will return{}if noconfigPathis found; - keep the name
tryLoadConfigbut throw a runtime error, and let the consumer handle the error;
What do you think?
It's a pity that there's no way to signal that a function can throw an error to a consumer :(
There was a problem hiding this comment.
Good call! I didn't spot the loadConfig opportunity (option 1) which we can definitely do. Went with that instead since throwing errors are a bit expensive.
Changes
Testing
Tested manually for config loading and restarting logic. The smoke tests should also cover this.
Docs
n/a. internal refactor.