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.

Sign up for GitHub

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

feat(astro): Exposes extra APIs for script and testing #12052

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat(astro): Exposes extra APIs for script and testing #12052

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3a73ea9
feat(astro): Exposes extra APIs for script and testing
Fryuni Sep 22, 2024
6da3161
Fix type lint issue
Fryuni Sep 22, 2024
b842f99
Merge remote-tracking branch 'origin/main' into feat/expose-apis-for-…
Fryuni Oct 8, 2024
7cac830
Update .changeset/bright-eels-cross.md
ematipico Oct 14, 2024
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
9 changes: 9 additions & 0 deletions .changeset/bright-eels-cross.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'astro': minor
---

Exposes extra APIs for scripting and testing:

- `mergeConfig` is now exported from `astro/config`, allowing users to merge Astro config objects directly;
- `validateConfig` is now exported from `astro/config`, alloweing users to resolve a config object into a valid `AstroConfig`;
ematipico marked this conversation as resolved.
Show resolved Hide resolved
- `build` API now includes the `teardownCompiler` option, allowing for more performant building during scripts and tests outside of Astro core.
21 changes: 21 additions & 0 deletions packages/astro/config.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
type ViteUserConfig = import('vite').UserConfig;
type ViteUserConfigFn = import('vite').UserConfigFn;
type AstroConfig = import('./dist/@types/astro.js').AstroConfig;
type AstroUserConfig = import('./dist/@types/astro.js').AstroUserConfig;
type AstroInlineConfig = import('./dist/@types/astro.js').AstroInlineConfig;
type ImageServiceConfig = import('./dist/@types/astro.js').ImageServiceConfig;
type SharpImageServiceConfig = import('./dist/assets/services/sharp.js').SharpImageServiceConfig;
type EnvField = typeof import('./dist/env/config.js').envField;
type DeepPartial<T> = import('./dist/type-utils.js').DeepPartial<T>;

/**
* See the full Astro Configuration API Documentation
Expand Down Expand Up @@ -46,3 +48,22 @@ export function passthroughImageService(): ImageServiceConfig;
* Return a valid env field to use in this Astro config for `experimental.env.schema`.
*/
export const envField: EnvField;

/**
* Recursively merges a partial configuration object into a valid default.
*
* Configuration can be merged both before and after being resolved.
*/
export function mergeConfig<C extends AstroConfig | AstroInlineConfig>(
defaults: C,
overrides: DeepPartial<C>,
bluwy marked this conversation as resolved.
Show resolved Hide resolved
): C;

/**
* Turn raw config values into normalized values
*/
export function validateConfig(
userConfig: any,
root: string,
cmd: string,
): Promise<AstroConfig>;
Comment on lines +62 to +69
Copy link
Member

Choose a reason for hiding this comment

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

What's the usecase for exporting this? I'm not too comfortable with exposing this yet since the validation doesn't take in account of --verbose and --silent (or log level in the future) and it always logs to the console when it shouldn't in tests, so it requires some more refactor.

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's one of the APIs necessary to implement testing properly. Otherwise, we'd need to re-implement the transformations fone in the schema.

It is mentioned at the end of the proposal:

Similarly, the function validateConfig that fills default values and apply transformations on the configuration is also not exported. This is required to resolve a value from the configuration that is not directly set on the test but used for the test tolling, like the outDir and srcDir.

it always logs to the console when it shouldn't in tests, so it requires some more refactor.

I don't see anywhere in the code where it would log anything. It creates a Zod schema and parses using it. None of the refine, superRefine and transform functions in the schema has any logs, they just add possible issues which will be reported as an error to the caller not as logs.

Are you perhaps thinking of resolveConfig?

Copy link
Member

Choose a reason for hiding this comment

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

Ah yeah it's resolveConfig that's logging the error, not validateConfig. But in that case, wouldn't resolveConfig be more useful as it mimics what dev() and build() loads for the astro config? validateConfig() only handles the validation part.

2 changes: 2 additions & 0 deletions packages/astro/config.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
export { defineConfig, getViteConfig } from './dist/config/index.js';
export { mergeConfig } from './dist/core/config/merge.js'
export { validateConfig } from './dist/core/config/validate.js'
export { envField } from './dist/env/config.js';

export function sharpImageService(config = {}) {
Expand Down
4 changes: 3 additions & 1 deletion packages/astro/src/core/build/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ export interface BuildOptions {
* Teardown the compiler WASM instance after build. This can improve performance when
* building once, but may cause a performance hit if building multiple times in a row.
*
* @internal only used for testing
* When building multiple projects in the same execution (e.g. during tests), disabling
* this option can greatly improve performance at the cost of some extra memory usage.
*
Comment on lines -43 to +45
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to keep this internal still. I imagine for external libraries this shouldn't have a significant impact if not used since it only mattered for Astro having tons of tests.

Copy link
Member Author

@Fryuni Fryuni Sep 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Astro has around 2,000 tests. For companies requiring high reliability, it is not uncommon to have very exhaustive testing on libraries (at least for the companies I know that fit that description, including mine).

Multiple of our libraries for Next have over 10k tests each. Libraries for Astro (which we are working on) won't be any less tested.

Copy link
Member

Choose a reason for hiding this comment

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

This isn't related to the number of test, but the number of builds for tests. Astro has many builds to test different permutations of all features and I doubt the same is needed for external libraries. Even so, Astro's test suite is due for some fixtures reduction because some fixtures could be combined, so in practice this option isn't really encouraged.

* @default true
*/
teardownCompiler?: boolean;
Expand Down
15 changes: 8 additions & 7 deletions packages/astro/src/core/config/merge.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { mergeConfig as mergeViteConfig } from 'vite';
import { mergeConfig as mergeViteConfig, type InlineConfig } from 'vite';
import { arraify, isObject, isURL } from '../util.js';
import type { AstroConfig, AstroInlineConfig } from '../../@types/astro.js';
import type { DeepPartial } from '../../type-utils.js';

function mergeConfigRecursively(
defaults: Record<string, any>,
Expand Down Expand Up @@ -64,10 +66,9 @@ function mergeConfigRecursively(
return merged;
}

export function mergeConfig(
defaults: Record<string, any>,
overrides: Record<string, any>,
isRoot = true,
): Record<string, any> {
return mergeConfigRecursively(defaults, overrides, isRoot ? '' : '.');
export function mergeConfig<C extends AstroConfig | AstroInlineConfig>(
defaults: C,
overrides: DeepPartial<C>,
): C {
return mergeConfigRecursively(defaults, overrides, '') as C;
}
11 changes: 1 addition & 10 deletions packages/astro/src/core/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,12 @@
// This is the main entrypoint when importing the `astro` package.

import type { AstroInlineConfig } from '../@types/astro.js';
import { default as _build } from './build/index.js';
import { default as _sync } from './sync/index.js';

export { default as build } from './build/index.js';
export { default as dev } from './dev/index.js';
export { default as preview } from './preview/index.js';

/**
* Builds your site for deployment. By default, this will generate static files and place them in a dist/ directory.
* If SSR is enabled, this will generate the necessary server files to serve your site.
*
* @experimental The JavaScript API is experimental
*/
// Wrap `_build` to prevent exposing the second internal options parameter
export const build = (inlineConfig: AstroInlineConfig) => _build(inlineConfig);

/**
* Generates TypeScript types for all Astro modules. This sets up a `src/env.d.ts` file for type inferencing,
* and defines the `astro:content` module for the Content Collections API.
Expand Down
5 changes: 3 additions & 2 deletions packages/astro/src/integrations/hooks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { AddressInfo } from 'node:net';
import { fileURLToPath } from 'node:url';
import { bold } from 'kleur/colors';
import type { InlineConfig, ViteDevServer } from 'vite';
import { mergeConfig as mergeViteConfig } from 'vite';
import type {
AstroAdapter,
AstroConfig,
Expand Down Expand Up @@ -183,7 +184,7 @@ export async function runHookConfigSetup({
updatedSettings.scripts.push({ stage, content });
},
updateConfig: (newConfig) => {
updatedConfig = mergeConfig(updatedConfig, newConfig) as AstroConfig;
updatedConfig = mergeConfig(updatedConfig, newConfig);
return { ...updatedConfig };
},
injectRoute: (injectRoute) => {
Expand Down Expand Up @@ -480,7 +481,7 @@ export async function runHookBuildSetup({
pages,
target,
updateConfig: (newConfig) => {
updatedConfig = mergeConfig(updatedConfig, newConfig);
updatedConfig = mergeViteConfig(updatedConfig, newConfig);
return { ...updatedConfig };
},
logger: getLogger(integration, logger),
Expand Down
Loading