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

Renderer plugins #231

Merged
merged 44 commits into from
May 26, 2021
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
e6d61e5
refactor: pluggable renderers
May 24, 2021
9ebe678
refactor: cache renderer per component
May 24, 2021
b34898d
docs: update comments on snowpack plugin `transform` method
May 24, 2021
8041f64
docs: add comments to renderer plugins
May 24, 2021
67f65c9
refactor: convert components to Map
May 24, 2021
096688e
fix: pass children through to astro __render
May 24, 2021
56637b9
refactor: move Components/ComponentInfo to shared types
May 24, 2021
05320b6
refactor: remove `gatherRuntimes` step, just scan output for imports
May 24, 2021
9a37889
refactor: update isComponentTag logic
May 24, 2021
9c723ec
chore: move dependencies to renderers
May 24, 2021
681fe05
fix: cross-platform transform injection
May 24, 2021
beca181
feat: defer renderer to react, fallback to preact
May 24, 2021
5daee3d
fix: use double quotes in generated script
May 24, 2021
f790d49
test: fix failing children tests
May 24, 2021
ab61569
test: add workspaceRoot to all tests
May 24, 2021
5d9e948
fix: pass props to renderer check
May 24, 2021
0927637
chore: add test:core script back for convenience
May 24, 2021
f5dd000
chore: remove unused external
May 25, 2021
94b0a8d
chore: rename renderers
May 25, 2021
4a06f5f
chore: add astring, estree-util-value-to-estree
May 25, 2021
d213115
chore: render-component => __astro_component
May 25, 2021
8a53669
refactor: split hydrate logic to own file
May 25, 2021
bf81f9a
refactor: use `astro-fragment` rather than `div`
May 25, 2021
b35755f
chore: remove unused hooks
May 25, 2021
1a0b8b8
chore: delete unused file
May 25, 2021
f47ec37
chore: add changesets
May 25, 2021
2f2a0f2
fix: Astro renderer should be async
May 25, 2021
a6f8f3a
fix: remove <astro-fragment> for static content
May 25, 2021
1f5444a
test: fix failing test
May 25, 2021
a2528a1
chore: normalize config interface
May 25, 2021
543fb22
feat: allow renderers to inject a snowpackPlugin
May 25, 2021
a9cdc7e
fix: resolve import URL before using dynamic import
May 25, 2021
fe7c92c
refactor: update renderers to use separate /server entrypoint
May 25, 2021
bc3c369
refactor: update server renderer interface
May 25, 2021
caf7252
fix: get renderers working again
May 25, 2021
8d584f9
test: debug failing test
May 26, 2021
40a73d0
test: better debug
May 26, 2021
c2c1f6b
test: better debug
May 26, 2021
3c27099
test: remove debug
May 26, 2021
76009ff
fix: support esm and cjs packages via "resolve"
May 26, 2021
9932312
refactor: split hydrate functions into individual files
May 26, 2021
9650538
fix: dependency resolution relative to projectRoot
May 26, 2021
7d979cc
fix: @snowpack/plugin-postcss needs to be hoisted
May 26, 2021
d530d90
fix: do not test prettier-plugin-astro as it's not ready for primetime
May 26, 2021
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
18 changes: 18 additions & 0 deletions .changeset/shaggy-countries-battle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
'astro': minor
---

**This is a breaking change**

Updated the rendering pipeline for `astro` to truly support any framework.

For the vast majority of use cases, `astro` should _just work_ out of the box. Astro now depends on `@astro-renderer/preact`, `@astro-renderer/react`, `@astro-renderer/svelte`, and `@astro-renderer/vue`, rather than these being built into the core library. This opens the door for anyone to contribute additional renderers for Astro to support their favorite framework, as well as the ability for users to control which renderers should be used.

**Features**
- Expose a pluggable interface for controlling server-side rendering and client-side hydration
- Allows components from different frameworks to be nested within each other.
> Note: `svelte` currently does support non-destructive hydration, so components from other frameworks cannot currently be nested inside of a Svelte component. See https://github.com/sveltejs/svelte/issues/4308.

**Breaking Changes**
- To improve compiler performance, improve framework support, and minimize JS payloads, any children passed to hydrated components are automatically wrapped with an `<astro-fragment>` element.
Copy link
Member

Choose a reason for hiding this comment

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

Artisanal changelog. Very nice 💯


8 changes: 8 additions & 0 deletions .changeset/smooth-toes-tan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@astro-renderer/preact': minor
'@astro-renderer/react': minor
'@astro-renderer/svelte': minor
'@astro-renderer/vue': minor
---

Initial release
6 changes: 0 additions & 6 deletions examples/kitchen-sink/astro.config.mjs

This file was deleted.

3 changes: 3 additions & 0 deletions examples/kitchen-sink/src/components/A.astro
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<div class="children">
<h1>Hello Astro (A)</h1>
</div>
3 changes: 3 additions & 0 deletions examples/kitchen-sink/src/components/B.astro
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<div class="children">
<h1>Hello Astro (B)</h1>
</div>
2 changes: 1 addition & 1 deletion examples/kitchen-sink/src/components/PreactCounter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { h, Fragment } from 'preact';
import { useState } from 'preact/hooks';

/** a counter written in Preact */
export default function PreactCounter({ children }) {
export function PreactCounter({ children }) {
const [count, setCount] = useState(0);
const add = () => setCount((i) => i + 1);
const subtract = () => setCount((i) => i - 1);
Expand Down
2 changes: 1 addition & 1 deletion examples/kitchen-sink/src/components/ReactCounter.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { useState } from 'react';

/** a counter written in React */
export default function ReactCounter({ children }) {
export function Counter({ children }) {
const [count, setCount] = useState(0);
const add = () => setCount((i) => i + 1);
const subtract = () => setCount((i) => i - 1);
Expand Down
2 changes: 2 additions & 0 deletions examples/kitchen-sink/src/components/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export { default as A } from './A.astro';
export { default as B } from './B.astro';
29 changes: 18 additions & 11 deletions examples/kitchen-sink/src/pages/index.astro
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
import ReactCounter from '../components/ReactCounter.jsx';
import PreactCounter from '../components/PreactCounter.tsx';
import { A, B as Renamed } from '../components';
import * as react from '../components/ReactCounter.jsx';
import { PreactCounter } from '../components/PreactCounter.tsx';
import VueCounter from '../components/VueCounter.vue';
import SvelteCounter from '../components/SvelteCounter.svelte';
---
Expand Down Expand Up @@ -30,22 +31,28 @@ import SvelteCounter from '../components/SvelteCounter.svelte';
</head>
<body>
<main>
<ReactCounter:load>

<react.Counter:visible>
<h1>Hello React!</h1>
<p>What's up?</p>
</ReactCounter:load>
</react.Counter:visible>
Comment on lines +35 to +38
Copy link
Contributor

@duncanhealy duncanhealy May 24, 2021

Choose a reason for hiding this comment

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

ReactCounter -> react.Counter will not match the COMPONENT_NAME_SCANNER regexp?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree with your suggestion to use a more complete RegExp that checks for validity, too.

But this does actually match! /^[A-Z]|\./ runs as (^[A-Z] OR \.) => starts with a capital letter OR has a period


<PreactCounter:load>
<h1>Hello Preact!</h1>
</PreactCounter:load>
<PreactCounter:visible>
<h1>Hello Preact!</h1>
</PreactCounter:visible>

<VueCounter:load>
<VueCounter:visible>
<h1>Hello Vue!</h1>
</VueCounter:load>
</VueCounter:visible>

<SvelteCounter:load>
<SvelteCounter:visible>
<h1>Hello Svelte!</h1>
</SvelteCounter:load>
</SvelteCounter:visible>

<A />

<Renamed />

</main>
</body>
</html>
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@
"dev:vscode": "lerna run dev --scope astro-languageserver --scope astro-vscode --scope astro-parser --parallel --stream",
"format": "prettier -w \"**/*.{js,jsx,ts,tsx,md,json}\"",
"lint": "eslint \"packages/**/*.ts\"",
"test": "lerna run test --scope astro --scope prettier-plugin-astro --scope create-astro --stream"
"test": "lerna run test --scope astro --scope prettier-plugin-astro --scope create-astro --stream",
"test:core": "yarn workspace astro run test"
},
"workspaces": [
"packages/*",
"packages/renderers/*",
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we gain anything by publishing these separately (given that we are depending on them in astro anyways)?

Copy link
Member

Choose a reason for hiding this comment

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

For now, no. But I think it’s safer to have them be external. We might have optional renderers in the future that require a manual install, maybe. This lets us have that without changing the setup.

"examples/*",
"tools/*",
"scripts",
Expand Down
15 changes: 6 additions & 9 deletions packages/astro/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,24 +31,27 @@
"test": "uvu test -i fixtures -i test-utils.js"
},
"dependencies": {
"@astro-renderer/preact": "0.0.1",
"@astro-renderer/react": "0.0.1",
"@astro-renderer/svelte": "0.0.1",
"@astro-renderer/vue": "0.0.1",
"@babel/code-frame": "^7.12.13",
"@babel/generator": "^7.13.9",
"@babel/parser": "^7.13.15",
"@babel/traverse": "^7.13.15",
"@silvenon/remark-smartypants": "^1.0.0",
"@snowpack/plugin-postcss": "^1.4.0",
"@snowpack/plugin-sass": "^1.4.0",
"@snowpack/plugin-svelte": "^3.7.0",
"@snowpack/plugin-vue": "^2.5.0",
"@vue/server-renderer": "^3.0.10",
"acorn": "^7.4.0",
"astring": "^1.7.4",
"astro-parser": "0.11.0",
"astro-prism": "0.0.2",
"autoprefixer": "^10.2.5",
"cheerio": "^1.0.0-rc.6",
"del": "^6.0.0",
"es-module-lexer": "^0.4.1",
"esbuild": "^0.10.1",
"estree-util-value-to-estree": "^1.2.0",
"estree-walker": "^3.0.0",
"fast-xml-parser": "^3.19.0",
"fdir": "^5.0.0",
Expand All @@ -68,11 +71,7 @@
"picomatch": "^2.2.3",
"postcss": "^8.2.15",
"postcss-icss-keyframes": "^0.2.1",
"preact": "^10.5.13",
"preact-render-to-string": "^5.1.18",
"prismjs": "^1.23.0",
"react": "^17.0.1",
"react-dom": "^17.0.1",
Copy link
Member

Choose a reason for hiding this comment

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

🙌🏻

"rehype-parse": "^7.0.1",
"rehype-raw": "^5.1.0",
"rehype-stringify": "^8.0.0",
Expand All @@ -88,10 +87,8 @@
"snowpack": "^3.5.1",
"source-map-support": "^0.5.19",
"string-width": "^5.0.0",
"svelte": "^3.35.0",
"tiny-glob": "^0.2.8",
"unified": "^9.2.1",
"vue": "^3.0.10",
"yargs-parser": "^20.2.7"
},
"devDependencies": {
Expand Down
37 changes: 34 additions & 3 deletions packages/astro/snowpack-plugin.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,53 @@ const { readFile } = require('fs').promises;
// Snowpack plugins must be CommonJS :(
const transformPromise = import('./dist/compiler/index.js');

module.exports = function (snowpackConfig, { resolvePackageUrl, extensions, astroConfig } = {}) {
module.exports = function (snowpackConfig, { resolvePackageUrl, renderers, astroConfig } = {}) {
return {
name: 'snowpack-astro',
knownEntrypoints: [],
resolve: {
input: ['.astro', '.md'],
output: ['.js', '.css'],
},
/**
* This injects our renderer plugins to the Astro runtime (as a bit of a hack).
*
* In a world where Snowpack supports virtual files, this won't be necessary and
* should be refactored to a virtual file that is imported by the runtime.
*
* Take a look at `/src/frontend/__astro_component.ts`. It relies on both
* `__rendererSources` and `__renderers` being defined, so we're creating those here.
*
* The output of this is the following (or something very close to it):
*
* ```js
* import * as __renderer_0 from '/_snowpack/link/packages/renderers/vue/index.js';
* import * as __renderer_1 from '/_snowpack/link/packages/renderers/svelte/index.js';
* import * as __renderer_2 from '/_snowpack/link/packages/renderers/preact/index.js';
* import * as __renderer_3 from '/_snowpack/link/packages/renderers/react/index.js';
* let __rendererSources = ["/_snowpack/link/packages/renderers/vue/client.js", "/_snowpack/link/packages/renderers/svelte/client.js", "/_snowpack/link/packages/renderers/preact/client.js", "/_snowpack/link/packages/renderers/react/client.js"];
* let __renderers = [__renderer_0, __renderer_1, __renderer_2, __renderer_3];
* // the original file contents
* ```
*/
async transform({contents, id, fileExt}) {
if (fileExt === '.js' && /__astro_component\.js/g.test(id)) {
const rendererServerPackages = await Promise.all(renderers.map(({ server }) => resolvePackageUrl(server)));
const rendererClientPackages = await Promise.all(renderers.map(({ client }) => resolvePackageUrl(client)));
const result = `${rendererServerPackages.map((pkg, i) => `import __renderer_${i} from "${pkg}";`).join('\n')}
let __rendererSources = [${rendererClientPackages.map(pkg => `"${pkg}"`).join(', ')}];
let __renderers = [${rendererServerPackages.map((_, i) => `__renderer_${i}`).join(', ')}];
${contents}`;
return result;
}
},
async load({ filePath }) {
const { compileComponent } = await transformPromise;
const projectRoot = snowpackConfig.root;
const contents = await readFile(filePath, 'utf-8');
const compileOptions = {
astroConfig,
resolvePackageUrl,
extensions,
renderers,
};
const result = await compileComponent(contents, { compileOptions, filename: filePath, projectRoot });
const output = {
Expand Down
13 changes: 10 additions & 3 deletions packages/astro/src/@types/astro.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import type { ImportSpecifier, ImportDefaultSpecifier, ImportNamespaceSpecifier } from '@babel/types';

export interface AstroConfigRaw {
dist: string;
projectRoot: string;
Expand All @@ -6,8 +8,6 @@ export interface AstroConfigRaw {
jsx?: string;
}

export type ValidExtensionPlugins = 'astro' | 'react' | 'preact' | 'svelte' | 'vue';

export interface AstroMarkdownOptions {
/** Enable or disable footnotes syntax extension */
footnotes: boolean;
Expand All @@ -19,7 +19,7 @@ export interface AstroConfig {
projectRoot: URL;
astroRoot: URL;
public: URL;
extensions?: Record<string, ValidExtensionPlugins>;
renderers?: string[];
/** Options for rendering markdown content */
markdownOptions?: Partial<AstroMarkdownOptions>;
/** Options specific to `astro build` */
Expand Down Expand Up @@ -171,3 +171,10 @@ export interface CollectionResult<T = any> {
/** Matched parameters, if any */
params: Params;
}

export interface ComponentInfo {
url: string;
importSpecifier: ImportSpecifier | ImportDefaultSpecifier | ImportNamespaceSpecifier;
}

export type Components = Map<string, ComponentInfo>;
3 changes: 1 addition & 2 deletions packages/astro/src/@types/compiler.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import type { LogOptions } from '../logger';
import type { AstroConfig, RuntimeMode, ValidExtensionPlugins } from './astro';
import type { AstroConfig, RuntimeMode } from './astro';

export interface CompileOptions {
logging: LogOptions;
resolvePackageUrl: (p: string) => Promise<string>;
astroConfig: AstroConfig;
extensions?: Record<string, ValidExtensionPlugins>;
Copy link
Member Author

Choose a reason for hiding this comment

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

No more extensions needed!

mode: RuntimeMode;
tailwindConfig?: string;
}
13 changes: 12 additions & 1 deletion packages/astro/src/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import fs from 'fs';
import path from 'path';
import { fileURLToPath } from 'url';
import { performance } from 'perf_hooks';
import eslexer from 'es-module-lexer';
import cheerio from 'cheerio';
import del from 'del';
import { bold, green, yellow } from 'kleur/colors';
Expand Down Expand Up @@ -94,6 +95,8 @@ export async function build(astroConfig: AstroConfig): Promise<0 | 1> {
// after pages are built, build depTree
timer.deps = performance.now();
const scanPromises: Promise<void>[] = [];

await eslexer.init;
for (const id of Object.keys(buildState)) {
if (buildState[id].contentType !== 'text/html') continue; // only scan HTML files
const pageDeps = findDeps(buildState[id].contents as string, {
Expand Down Expand Up @@ -237,8 +240,16 @@ export function findDeps(html: string, { astroConfig, srcPath }: { astroConfig:

$('script').each((i, el) => {
const src = $(el).attr('src');
if (src && !isRemote(src)) {
if (src) {
if (isRemote(src)) return;
pageDeps.js.add(getDistPath(src, { astroConfig, srcPath }));
} else {
const text = $(el).html();
if (!text) return;
const [imports] = eslexer.parse(text);
for (const spec of imports) {
if (spec.n) pageDeps.js.add(spec.n);
Copy link
Member

Choose a reason for hiding this comment

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

I think we may need to resolve these specifiers with getDistPath()? What that does is try and turn relative imports into their absolute paths before we lose the context of where each was loaded from.

Copy link
Member Author

@natemoo-re natemoo-re May 26, 2021

Choose a reason for hiding this comment

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

Ah good callout. The script generated by the renderers is already pointing to the dist path. Do I need to check if an import is relative before passing it through getDistPath() or does it handle that for me?

Copy link
Member

Choose a reason for hiding this comment

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

If it’s already absolute then you don’t need to bother, but getDistPath should ignore absolute URLs. Feel free to ignore in this PR; we probably have some more testing IRT user-created inline scripts that are unrelated to rendering

}
}
});

Expand Down
Loading