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 configs with Vite when loading with Proload fails #4112

Merged
merged 8 commits into from
Aug 2, 2022
Merged
Show file tree
Hide file tree
Changes from 7 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
6 changes: 6 additions & 0 deletions .changeset/dull-radios-sparkle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'astro': patch
'@astrojs/mdx': patch
---

Fix MDX working with a ts config file
79 changes: 49 additions & 30 deletions packages/astro/src/core/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { z } from 'zod';
import { LogOptions } from './logger/core.js';
import { appendForwardSlash, prependForwardSlash, trimSlashes } from './path.js';
import { arraify, isObject } from './util.js';
import * as vite from 'vite';

load.use([loadTypeScript]);

Expand Down Expand Up @@ -409,10 +410,13 @@ export async function resolveConfigURL(
const flags = resolveFlags(configOptions.flags || {});
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`
const configPath = await resolve('astro', {
Expand Down Expand Up @@ -447,21 +451,7 @@ export async function openConfig(configOptions: LoadConfigOptions): Promise<Open
);
}

// Automatically load config file using Proload
// If `userConfigPath` is `undefined`, Proload will search for `astro.config.[cm]?[jt]s`
let config;
try {
config = await load('astro', {
mustExist: !!userConfigPath,
cwd: root,
filePath: userConfigPath,
});
} catch (err) {
if (err instanceof ProloadError && flags.config) {
throw new Error(`Unable to resolve --config "${flags.config}"! Does the file exist?`);
}
throw err;
}
const config = await tryLoadConfig(configOptions, flags, userConfigPath, root);
if (config) {
userConfig = config.value;
userConfigPath = config.filePath;
Expand All @@ -483,6 +473,49 @@ export async function openConfig(configOptions: LoadConfigOptions): Promise<Open
};
}

interface TryLoadConfigResult {
value: Record<string, any>;
filePath?: string;
}

async function tryLoadConfig(configOptions: LoadConfigOptions, flags: CLIFlags, userConfigPath: string | undefined, root: string): Promise<TryLoadConfigResult | undefined> {
try {
// Automatically load config file using Proload
// If `userConfigPath` is `undefined`, Proload will search for `astro.config.[cm]?[jt]s`
const config = await load('astro', {
mustExist: !!userConfigPath,
cwd: root,
filePath: userConfigPath,
});

return config as TryLoadConfigResult;
} catch(e) {
if (e instanceof ProloadError && flags.config) {
Copy link
Member

Choose a reason for hiding this comment

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

does this mean that --config won't work with .ts config files? Could this be moved to further down to be a final catchall, OR earlier in the function with a fs.stat call to check that the file exists before running proload or vite?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this condition hits when the user passes a --config but that file doesn't exist. If the file does exist and is a .ts file it will either a) have worked through proload or b) caught here, but the instanceof check will fail and it will load through the subsequent Vite mechanism.

throw new Error(`Unable to resolve --config "${flags.config}"! Does the file exist?`);
}

const configURL = await resolveConfigURL(configOptions);
if(!configURL) {
throw e;
}

// Fallback to use Vite DevServer
const viteServer = await vite.createServer({});
bholmesdev marked this conversation as resolved.
Show resolved Hide resolved
try {
const mod = await viteServer.ssrLoadModule(fileURLToPath(configURL));

if(mod?.default) {
return {
value: mod.default,
filePath: fileURLToPath(configURL),
};
};
} finally {
await viteServer.close();
}
}
}

/**
* Attempt to load an `astro.config.mjs` file
* @deprecated
Expand All @@ -500,21 +533,7 @@ export async function loadConfig(configOptions: LoadConfigOptions): Promise<Astr
);
}

// Automatically load config file using Proload
// If `userConfigPath` is `undefined`, Proload will search for `astro.config.[cm]?[jt]s`
let config;
try {
config = await load('astro', {
mustExist: !!userConfigPath,
cwd: root,
filePath: userConfigPath,
});
} catch (err) {
if (err instanceof ProloadError && flags.config) {
throw new Error(`Unable to resolve --config "${flags.config}"! Does the file exist?`);
}
throw err;
}
const config = await tryLoadConfig(configOptions, flags, userConfigPath, root);
if (config) {
userConfig = config.value;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/integrations/mdx/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export default function mdx(mdxOptions: MdxOptions = {}): AstroIntegration {
// Workarounds tried:
// - "import * as remarkShikiTwoslash"
// - "import { default as remarkShikiTwoslash }"
(remarkShikiTwoslash as any).default,
(remarkShikiTwoslash as any).default ?? remarkShikiTwoslash,
Copy link
Contributor

Choose a reason for hiding this comment

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

Good thinking here. Wonder if proload was somehow messing with the .default here...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's not why proload was failing. But this is needed to be compatible with loading in Vite.

config.markdown.shikiConfig,
]);
rehypePlugins.push([rehypeRaw, { passThrough: nodeTypes }]);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import mdx from '@astrojs/mdx';

export default {
integrations: [mdx()]
}
7 changes: 7 additions & 0 deletions packages/integrations/mdx/test/fixtures/mdx-page/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "@test/mdx-page",
"dependencies": {
"astro": "workspace:*",
"@astrojs/mdx": "workspace:*"
}
}
3 changes: 1 addition & 2 deletions packages/integrations/mdx/test/mdx-page.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ describe('MDX Page', () => {

before(async () => {
fixture = await loadFixture({
root: new URL('./fixtures/mdx-page/', import.meta.url),
integrations: [mdx()],
root: new URL('./fixtures/mdx-page/', import.meta.url)
});
});

Expand Down
8 changes: 8 additions & 0 deletions pnpm-lock.yaml

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