Skip to content

Commit

Permalink
Merge branch 'withastro:main' into main
Browse files Browse the repository at this point in the history
  • Loading branch information
alex-sherwin authored and Alex Sherwin committed May 30, 2023
2 parents 7b4d832 + e20a3b2 commit 6cdba3c
Show file tree
Hide file tree
Showing 21 changed files with 338 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .changeset/small-wombats-know.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

fix miss a head when the templaterender has a promise
5 changes: 5 additions & 0 deletions .changeset/strange-socks-give.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Use `AstroError` for `Astro.glob` errors
23 changes: 21 additions & 2 deletions packages/astro/e2e/errors.test.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,21 @@
import { expect } from '@playwright/test';
import { getErrorOverlayContent, testFactory } from './test-utils.js';
import { getErrorOverlayContent, silentLogging, testFactory } from './test-utils.js';

const test = testFactory({
root: './fixtures/errors/',
// Only test the error overlay, don't print to console
vite: {
logLevel: 'silent',
},
});

let devServer;

test.beforeAll(async ({ astro }) => {
devServer = await astro.startDevServer();
devServer = await astro.startDevServer({
// Only test the error overlay, don't print to console
logging: silentLogging,
});
});

test.afterAll(async ({ astro }) => {
Expand Down Expand Up @@ -89,4 +96,16 @@ test.describe('Error display', () => {

expect(await page.locator('vite-error-overlay').count()).toEqual(0);
});

test('astro glob no match error', async ({ page, astro }) => {
await page.goto(astro.resolveUrl('/astro-glob-no-match'), { waitUntil: 'networkidle' });
const message = (await getErrorOverlayContent(page)).message;
expect(message).toMatch('did not return any matching files');
});

test('astro glob used outside of an astro file', async ({ page, astro }) => {
await page.goto(astro.resolveUrl('/astro-glob-outside-astro'), { waitUntil: 'networkidle' });
const message = (await getErrorOverlayContent(page)).message;
expect(message).toMatch('can only be used in');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function globSomething(Astro) {
return Astro.glob('./*.lua')
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
---
Astro.glob('./*.lua')
---
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
import { globSomething } from '../components/AstroGlobOutsideAstro'
globSomething(Astro)
---
2 changes: 2 additions & 0 deletions packages/astro/e2e/test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { loadFixture as baseLoadFixture } from '../test/test-utils.js';

export const isWindows = process.platform === 'win32';

export { silentLogging } from '../test/test-utils.js';

// Get all test files in directory, assign unique port for each of them so they don't conflict
const testFiles = await fs.readdir(new URL('.', import.meta.url));
const testFileToPort = new Map();
Expand Down
30 changes: 30 additions & 0 deletions packages/astro/src/core/errors/errors-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,36 @@ See https://docs.astro.build/en/guides/server-side-rendering/ for more informati
message: (imageFilePath: string) =>
`\`Image\`'s and \`getImage\`'s \`src\` parameter must be an imported image or an URL, it cannot be a filepath. Received \`${imageFilePath}\`.`,
},

/**
* @docs
* @see
* - [Astro.glob](https://docs.astro.build/en/reference/api-reference/#astroglob)
* @description
* `Astro.glob()` can only be used in `.astro` files. You can use [`import.meta.glob()`](https://vitejs.dev/guide/features.html#glob-import) instead to acheive the same result.
*/
AstroGlobUsedOutside: {
title: 'Astro.glob() used outside of an Astro file.',
code: 3035,
message: (globStr: string) =>
`\`Astro.glob(${globStr})\` can only be used in \`.astro\` files. \`import.meta.glob(${globStr})\` can be used instead to achieve a similar result.`,
hint: "See Vite's documentation on `import.meta.glob` for more information: https://vitejs.dev/guide/features.html#glob-import",
},

/**
* @docs
* @see
* - [Astro.glob](https://docs.astro.build/en/reference/api-reference/#astroglob)
* @description
* `Astro.glob()` did not return any matching files. There might be a typo in the glob pattern.
*/
AstroGlobNoMatch: {
title: 'Astro.glob() did not match any files.',
code: 3036,
message: (globStr: string) =>
`\`Astro.glob(${globStr})\` did not return any matching files. Check the pattern for typos.`,
},

// No headings here, that way Vite errors are merged with Astro ones in the docs, which makes more sense to users.
// Vite Errors - 4xxx
/**
Expand Down
13 changes: 9 additions & 4 deletions packages/astro/src/runtime/server/astro-global.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
import type { AstroGlobalPartial } from '../../@types/astro';
import { ASTRO_VERSION } from '../../core/constants.js';
import { AstroError, AstroErrorData } from '../../core/errors/index.js';

/** Create the Astro.glob() runtime function. */
function createAstroGlobFn() {
const globHandler = (importMetaGlobResult: Record<string, any>, globValue: () => any) => {
if (typeof importMetaGlobResult === 'string') {
throw new Error(
'Astro.glob() does not work outside of an Astro file. Use `import.meta.glob()` instead.'
);
throw new AstroError({
...AstroErrorData.AstroGlobUsedOutside,
message: AstroErrorData.AstroGlobUsedOutside.message(JSON.stringify(importMetaGlobResult)),
});
}
let allEntries = [...Object.values(importMetaGlobResult)];
if (allEntries.length === 0) {
throw new Error(`Astro.glob(${JSON.stringify(globValue())}) - no matches found.`);
throw new AstroError({
...AstroErrorData.AstroGlobNoMatch,
message: AstroErrorData.AstroGlobNoMatch.message(JSON.stringify(importMetaGlobResult)),
});
}
// Map over the `import()` promises, calling to load them.
return Promise.all(allEntries.map((fn) => fn()));
Expand Down
3 changes: 3 additions & 0 deletions packages/astro/src/runtime/server/render/astro/instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ export class AstroComponentInstance {
value = await value;
}
if (isHeadAndContent(value)) {
if (this.result.extraHead.length === 0 && value.head) {
yield renderChild(value.head);
}
yield* value.content;
} else {
yield* renderChild(value);
Expand Down
11 changes: 8 additions & 3 deletions packages/astro/test/units/runtime/astro-global.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,13 @@ describe('astro global', () => {
const Astro = createAstro(undefined);
expect(() => {
Astro.glob('./**/*.md');
}).to.throw(
'Astro.glob() does not work outside of an Astro file. Use `import.meta.glob()` instead.'
);
}).to.throw(/can only be used in/);
});

it('Glob should error if has no results', async () => {
const Astro = createAstro(undefined);
expect(() => {
Astro.glob([], () => './**/*.md');
}).to.throw(/did not return any matching files/);
});
});
8 changes: 4 additions & 4 deletions packages/integrations/markdoc/test/image-assets.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,17 @@ describe('Markdoc - Image assets', () => {
const res = await baseFixture.fetch('/');
const html = await res.text();
const { document } = parseHTML(html);
expect(document.querySelector('#relative > img')?.src).to.equal(
'/_image?href=%2Fsrc%2Fassets%2Frelative%2Foar.jpg%3ForigWidth%3D420%26origHeight%3D630%26origFormat%3Djpg&f=webp'
expect(document.querySelector('#relative > img')?.src).to.match(
/\/_image\?href=.*%2Fsrc%2Fassets%2Frelative%2Foar.jpg%3ForigWidth%3D420%26origHeight%3D630%26origFormat%3Djpg&f=webp/
);
});

it('transforms aliased image paths to optimized path', async () => {
const res = await baseFixture.fetch('/');
const html = await res.text();
const { document } = parseHTML(html);
expect(document.querySelector('#alias > img')?.src).to.equal(
'/_image?href=%2Fsrc%2Fassets%2Falias%2Fcityscape.jpg%3ForigWidth%3D420%26origHeight%3D280%26origFormat%3Djpg&f=webp'
expect(document.querySelector('#alias > img')?.src).to.match(
/\/_image\?href=.*%2Fsrc%2Fassets%2Falias%2Fcityscape.jpg%3ForigWidth%3D420%26origHeight%3D280%26origFormat%3Djpg&f=webp/
);
});
});
Expand Down
107 changes: 107 additions & 0 deletions packages/integrations/mdx/src/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
# Internal documentation

## rehype-optimize-static

The `rehype-optimize-static` plugin helps optimize the intermediate [`hast`](https://github.com/syntax-tree/hast) when processing MDX, collapsing static subtrees of the `hast` as a `"static string"` in the final JSX output. Here's a "before" and "after" result:

Before:

```jsx
function _createMdxContent() {
return (
<>
<h1>My MDX Content</h1>
<pre>
<code class="language-js">
<span class="token function">console</span>
<span class="token punctuation">.</span>
<span class="token function">log</span>
<span class="token punctuation">(</span>
<span class="token string">'hello world'</span>
<span class="token punctuation">)</span>
</code>
</pre>
</>
);
}
```

After:

```jsx
function _createMdxContent() {
return (
<>
<h1>My MDX Content</h1>
<pre set:html="<code class=...</code>"></pre>
</>
);
}
```

> NOTE: If one of the nodes in `pre` is MDX, the optimization will not be applied to `pre`, but could be applied to the inner MDX node if its children are static.
This results in fewer JSX nodes, less compiled JS output, and less parsed AST, which results in faster Rollup builds and runtime rendering.

To acheive this, we use an algorithm to detect `hast` subtrees that are entirely static (containing no JSX) to be inlined as `set:html` to the root of the subtree.

The next section explains the algorithm, which you can follow along by pairing with the [source code](./rehype-optimize-static.ts). To analyze the `hast`, you can paste the MDX code into https://mdxjs.com/playground.

### How it works

Two variables:

- `allPossibleElements`: A set of subtree roots where we can add a new `set:html` property with its children as value.
- `elementStack`: The stack of elements (that could be subtree roots) while traversing the `hast` (node ancestors).

Flow:

1. Walk the `hast` tree.
2. For each `node` we enter, if the `node` is static (`type` is `element` or `mdxJsxFlowElement`), record in `allPossibleElements` and push to `elementStack`.
- Q: Why do we record `mdxJsxFlowElement`, it's MDX? <br>
A: Because we're looking for nodes whose children are static. The node itself doesn't need to be static.
- Q: Are we sure this is the subtree root node in `allPossibleElements`? <br>
A: No, but we'll clear that up later in step 3.
3. For each `node` we leave, pop from `elementStack`. If the `node`'s parent is in `allPossibleElements`, we also remove the `node` from `allPossibleElements`.
- Q: Why do we check for the node's parent? <br>
A: Checking for the node's parent allows us to identify a subtree root. When we enter a subtree like `C -> D -> E`, we leave in reverse: `E -> D -> C`. When we leave `E`, we see that it's parent `D` exists, so we remove `E`. When we leave `D`, we see `C` exists, so we remove `D`. When we leave `C`, we see that its parent doesn't exist, so we keep `C`, a subtree root.
4. _(Returning to the code written for step 2's `node` enter handling)_ We also need to handle the case where we find non-static elements. If found, we remove all the elements in `elementStack` from `allPossibleElements`. This happens before the code in step 2.
- Q: Why? <br>
A: Because if the `node` isn't static, that means all its ancestors (`elementStack`) have non-static children. So, the ancestors couldn't be a subtree root to be optimized anymore.
- Q: Why before step 2's `node` enter handling? <br>
A: If we find a non-static `node`, the `node` should still be considered in `allPossibleElements` as its children could be static.
5. Walk done. This leaves us with `allPossibleElements` containing only subtree roots that can be optimized.
6. Add the `set:html` property to the `hast` node, and remove its children.
7. 🎉 The rest of the MDX pipeline will do its thing and generate the desired JSX like above.

### Extra

#### MDX custom components

Astro's MDX implementation supports specifying `export const components` in the MDX file to render some HTML elements as Astro components or framework components. `rehype-optimize-static` also needs to parse this JS to recognize some elements as non-static.

#### Further optimizations

In [How it works](#how-it-works) step 4,

> we remove all the elements in `elementStack` from `allPossibleElements`
We can further optimize this by then also emptying the `elementStack`. This ensures that if we run this same flow for a deeper node in the tree, we don't remove the already-removed nodes from `allPossibleElements`.

While this breaks the concept of `elementStack`, it doesn't matter as the `elementStack` array pop in the "leave" handler (in step 3) would become a no-op.

Example `elementStack` value during walking phase:

```
Enter: A
Enter: A, B
Enter: A, B, C
(Non-static node found): <empty>
Enter: D
Enter: D, E
Leave: D
Leave: <empty>
Leave: <empty>
Leave: <empty>
Leave: <empty>
```
49 changes: 49 additions & 0 deletions packages/integrations/mdx/test/astro-content-css.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { expect } from 'chai';
import * as cheerio from 'cheerio';
import { loadFixture } from '../../../astro/test/test-utils.js';
import mdx from '@astrojs/mdx';

describe('build css from the component', async () => {
let fixture;

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

describe('Build', () => {
before(async () => {
await fixture.build();
});

it('including css and js from the component in pro', async () => {
const html = await fixture.readFile('/index.html');
const $ = cheerio.load(html);
expect($('link[href$=".css"]').attr('href')).to.match(/^\/_astro\//);
expect($('script[src$=".js"]').attr('src')).to.match(/^\/_astro\//);
});
});

describe('Dev', () => {
let devServer;
before(async () => {
devServer = await fixture.startDevServer();
});

after(async () => {
devServer.stop();
});

it('ncluding css and js from the component in Dev', async () => {
let res = await fixture.fetch(`/`);
expect(res.status).to.equal(200);
const html = await res.text();
const $ = cheerio.load(html);
expect($.html()).to.include('CornflowerBlue');
expect($('script[src$=".js"]').attr('src')).to.include('astro');
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { defineConfig } from 'astro/config';

import mdx from "@astrojs/mdx";

// https://astro.build/config
export default defineConfig({
build: {
format: 'file'
},
integrations: [mdx()]
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "@test/astro-content-css",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*",
"@astrojs/mdx": "workspace:*"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// 1. Import utilities from `astro:content`
import { z, defineCollection } from 'astro:content';
// 2. Define a schema for each collection you'd like to validate.
const dynamicCollection = defineCollection({
schema: z.object({
title: z.string(),
}),
});
// 3. Export a single `collections` object to register your collection(s)
export const collections = {
dynamic: dynamicCollection,
};
Loading

0 comments on commit 6cdba3c

Please sign in to comment.