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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/calm-peas-doubt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': major
---

Remove proload to load the Astro config. It will now use NodeJS and Vite to load the config only.
2 changes: 0 additions & 2 deletions packages/astro/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,6 @@
"@babel/plugin-transform-react-jsx": "^7.17.12",
"@babel/traverse": "^7.18.2",
"@babel/types": "^7.18.4",
"@proload/core": "^0.3.3",
"@proload/plugin-tsm": "^0.2.1",
"@types/babel__core": "^7.1.19",
"@types/html-escaper": "^3.0.0",
"@types/yargs-parser": "^21.0.0",
Expand Down
4 changes: 0 additions & 4 deletions packages/astro/src/content/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ export async function loadContentConfig({
settings: AstroSettings;
}): Promise<ContentConfig | Error> {
const contentPaths = getContentPaths({ srcDir: settings.config.srcDir });
const nodeEnv = process.env.NODE_ENV;
const tempConfigServer: ViteDevServer = await createServer({
root: fileURLToPath(settings.config.root),
server: { middlewareMode: true, hmr: false },
Expand All @@ -207,9 +206,6 @@ export async function loadContentConfig({
return new NotFoundError('Failed to resolve content config.');
} finally {
await tempConfigServer.close();
// Reset NODE_ENV to initial value
// Vite's `createServer()` sets NODE_ENV to 'development'!
process.env.NODE_ENV = nodeEnv;
Comment on lines -210 to -212
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.

}
const config = contentConfigParser.safeParse(unparsedConfig);
if (config.success) {
Expand Down
68 changes: 32 additions & 36 deletions packages/astro/src/core/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export function resolveRoot(cwd?: string | URL): string {
}

/** Merge CLI flags & user config object (CLI flags take priority) */
function mergeCLIFlags(astroConfig: AstroUserConfig, flags: CLIFlags, cmd: string) {
function mergeCLIFlags(astroConfig: AstroUserConfig, flags: CLIFlags) {
astroConfig.server = astroConfig.server || {};
astroConfig.markdown = astroConfig.markdown || {};
astroConfig.experimental = astroConfig.experimental || {};
Expand All @@ -136,6 +136,23 @@ function mergeCLIFlags(astroConfig: AstroUserConfig, flags: CLIFlags, cmd: strin
return astroConfig;
}

async function search(fsMod: typeof fs, root: string) {
const paths = [
'astro.config.mjs',
'astro.config.js',
'astro.config.ts',
'astro.config.mts',
'astro.config.cjs',
'astro.config.cts',
].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.

return file;
}
}
}

interface LoadConfigOptions {
cwd?: string;
flags?: Flags;
Expand All @@ -147,41 +164,36 @@ interface LoadConfigOptions {
fsMod?: typeof fs;
}

interface ResolveConfigPathOptions {
cwd?: string;
flags?: Flags;
fs: typeof fs;
}

/**
* Resolve the file URL of the user's `astro.config.js|cjs|mjs|ts` file
* Note: currently the same as loadConfig but only returns the `filePath`
* instead of the resolved config
*/
export async function resolveConfigPath(
configOptions: Pick<LoadConfigOptions, 'cwd' | 'flags'> & { fs: typeof fs }
configOptions: ResolveConfigPathOptions
): Promise<string | undefined> {
const root = resolveRoot(configOptions.cwd);
const flags = resolveFlags(configOptions.flags || {});
let userConfigPath: string | undefined;

let userConfigPath: string | undefined;
if (flags?.config) {
userConfigPath = /^\.*\//.test(flags.config) ? flags.config : `./${flags.config}`;
userConfigPath = fileURLToPath(new URL(userConfigPath, `file://${root}/`));
}

// Resolve config file path using Proload
// If `userConfigPath` is `undefined`, Proload will search for `astro.config.[cm]?[jt]s`
try {
const config = await loadConfigWithVite({
configPath: userConfigPath,
root,
fs: configOptions.fs,
});
return config.filePath;
} catch (e) {
if (flags.config) {
if (!configOptions.fs.existsSync(userConfigPath)) {
throw new AstroError({
...AstroErrorData.ConfigNotFound,
message: AstroErrorData.ConfigNotFound.message(flags.config),
});
}
throw e;
} else {
userConfigPath = await search(configOptions.fs, root);
}

return userConfigPath;
}

interface OpenConfigResult {
Expand Down Expand Up @@ -261,30 +273,14 @@ async function tryLoadConfig(
}
}

/**
* Attempt to load an `astro.config.mjs` file
* @deprecated
*/
export async function loadConfig(configOptions: LoadConfigOptions): Promise<AstroConfig> {
const root = resolveRoot(configOptions.cwd);
const flags = resolveFlags(configOptions.flags || {});
let userConfig: AstroUserConfig = {};

const config = await tryLoadConfig(configOptions, root);
if (config) {
userConfig = config.value;
}
return resolveConfig(userConfig, root, flags, configOptions.cmd);
}

/** Attempt to resolve an Astro configuration object. Normalize, validate, and return. */
export async function resolveConfig(
userConfig: AstroUserConfig,
root: string,
flags: CLIFlags = {},
cmd: string
): Promise<AstroConfig> {
const mergedConfig = mergeCLIFlags(userConfig, flags, cmd);
const mergedConfig = mergeCLIFlags(userConfig, flags);
const validatedConfig = await validateConfig(mergedConfig, root, cmd);

return validatedConfig;
Expand Down
92 changes: 14 additions & 78 deletions packages/astro/src/core/config/vite-load.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,7 @@
import type fsType from 'fs';
import npath from 'path';
import { pathToFileURL } from 'url';
import * as vite from 'vite';
import loadFallbackPlugin from '../../vite-plugin-load-fallback/index.js';
import { AstroError, AstroErrorData } from '../errors/index.js';

// Fallback for legacy
import load from '@proload/core';
import loadTypeScript from '@proload/plugin-tsm';

load.use([loadTypeScript]);

export interface ViteLoader {
root: string;
Expand All @@ -24,8 +16,8 @@ async function createViteLoader(root: string, fs: typeof fsType): Promise<ViteLo
appType: 'custom',
ssr: {
// NOTE: Vite doesn't externalize linked packages by default. During testing locally,
// these dependencies trip up Vite's dev SSR transform. In the future, we should
// avoid `vite.createServer` and use `loadConfigFromFile` instead.
// these dependencies trip up Vite's dev SSR transform. Awaiting upstream feature:
// https://github.com/vitejs/vite/pull/10939
external: [
'@astrojs/tailwind',
'@astrojs/mdx',
Expand All @@ -43,40 +35,6 @@ async function createViteLoader(root: string, fs: typeof fsType): Promise<ViteLo
};
}

async function stat(fs: typeof fsType, configPath: string, mustExist: boolean): Promise<boolean> {
try {
await fs.promises.stat(configPath);
return true;
} catch {
if (mustExist) {
throw new AstroError({
...AstroErrorData.ConfigNotFound,
message: AstroErrorData.ConfigNotFound.message(configPath),
});
}
return false;
}
}

async function search(fs: typeof fsType, root: string) {
const paths = [
'astro.config.mjs',
'astro.config.js',
'astro.config.ts',
'astro.config.mts',
'astro.config.cjs',
'astro.config.cjs',
].map((path) => npath.join(root, path));

for (const file of paths) {
// First verify the file event exists
const exists = await stat(fs, file, false);
if (exists) {
return file;
}
}
}

interface LoadConfigWithViteOptions {
root: string;
configPath: string | undefined;
Expand All @@ -91,58 +49,36 @@ export async function loadConfigWithVite({
value: Record<string, any>;
filePath?: string;
}> {
let file: string;
if (configPath) {
// Go ahead and check if the file exists and throw if not.
await stat(fs, configPath, true);
file = configPath;
} else {
const found = await search(fs, root);
if (!found) {
// No config file found, return an empty config that will be populated with defaults
return {
value: {},
filePath: undefined,
};
} else {
file = found;
}
// No config file found, return an empty config that will be populated with defaults
if (!configPath) {
return {
value: {},
filePath: undefined,
};
}

// Try loading with Node import()
if (/\.[cm]?js$/.test(file)) {
if (/\.[cm]?js$/.test(configPath)) {
try {
const config = await import(pathToFileURL(file).toString());
const config = await import(pathToFileURL(configPath).toString());
return {
value: config.default ?? {},
filePath: file,
filePath: configPath,
};
} catch {
// We do not need to keep the error here because with fallback the error will be rethrown
// when/if it fails in Proload.
// when/if it fails in Vite.
}
}

// Try Loading with Vite
let loader: ViteLoader | undefined;
try {
loader = await createViteLoader(root, fs);
const mod = await loader.viteServer.ssrLoadModule(file);
const mod = await loader.viteServer.ssrLoadModule(configPath);
return {
value: mod.default ?? {},
filePath: file,
};
} catch {
// Try loading with Proload
// TODO deprecate - this is only for legacy compatibility
const res = await load('astro', {
mustExist: true,
cwd: root,
filePath: file,
});
return {
value: res?.value ?? {},
filePath: file,
filePath: configPath,
};
} finally {
if (loader) {
Expand Down
8 changes: 6 additions & 2 deletions packages/astro/test/test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import stripAnsi from 'strip-ansi';
import { fileURLToPath } from 'url';
import { sync } from '../dist/cli/sync/index.js';
import build from '../dist/core/build/index.js';
import { loadConfig } from '../dist/core/config/config.js';
import { openConfig } from '../dist/core/config/config.js';
import { createSettings } from '../dist/core/config/index.js';
import dev from '../dist/core/dev/index.js';
import { nodeLogDestination } from '../dist/core/logger/node.js';
Expand Down Expand Up @@ -86,7 +86,11 @@ export async function loadFixture(inlineConfig) {
const logging = defaultLogging;

// Load the config.
let config = await loadConfig({ cwd: fileURLToPath(cwd), logging });
let { astroConfig: config } = await openConfig({
cwd: fileURLToPath(cwd),
logging,
cmd: 'dev',
});
config = merge(config, { ...inlineConfig, root: cwd });

// HACK: the inline config doesn't run through config validation where these normalizations usually occur
Expand Down
38 changes: 0 additions & 38 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading