WIP: renderer plugins #155

natemoo-re · 2021-04-30T14:32:42Z

Todo

Changes

Follow up to #74, this PR is a WIP attempt at a possible renderer plugin interface. This approach is similar to the one we currently have but more robust. There's a ton of work left to get these integrated into the actual build process, but this feels like a good foundation.

Here is the current preact interface:

import type { AstroRenderer } from 'astro/renderer';
import type { ComponentType } from 'preact';

// Optionally provide types for the client/server dependencies
interface PreactDependencies {
  // Dependencies of both the client AND server (usually the JSX `importSource`)
  shared: {
    ['preact']: typeof import('preact')
  }
  // Dependencies for the server (usually the server rendering package)
  server: {
    ['preact-render-to-string']: typeof import('preact-render-to-string')
  },
  // Dependencies for the client (`{}` is empty)
  client: {}
}

const validExtensions = new Set(['.jsx', '.tsx']);

const renderer: AstroRenderer<PreactDependencies, ComponentType> = {

  // Renderers can claim a file based on the file type
  // or some metadata (TBD) about the file's contents (if necessary)
  filter(id, { imports }) {
    const ext = id.slice(0, -4);
    if (!validExtensions.has(ext)) return;
    if (!imports.has('preact')) return;
    return true;
  },

  // JSX related configuration, needed for frameworks that require `children` to be JSX
  jsx: {
    importSource: 'preact',
    factory: 'h',
    fragmentFactory: 'Fragment',
    // should `children` be converted to JSX?
    transformChildren: true,
  },

  server: {
    dependencies: ['preact', 'preact-render-to-string'],
    renderToStaticMarkup({ preact, ['preact-render-to-string']: preactRenderToString }) {
      const { h } = preact;
      const { renderToString } = preactRenderToString;
      // Must return a factory function that returns any generated content 
      return async (Component, props, children) => {
        const code = renderToString(h(Component, props, children));

        // `{ [fileType: string] : { code: string, map?: SourceMap }`
        return { 
          '.html': { code }
        };
      };
    },
  },

  client: {
    dependencies: ['preact'],
    hydrateStaticMarkup({ preact }, el) {
      // Must return a factory function that returns JavaScript to be run on the client (as a string)
      // Function parameters are _references_ to the values, not the values themselves
      return (Component, props, children) => `
        const {h,hydrate} = ${preact};
        hydrate(h(${Component},${props},${children}),${el})
      `;
    },
  },
};

export default renderer;

For comparison, here is the proposed svelte interface which doesn't rely on JSX but does use a local wrapper to use a JSX-like function signature.

import type { AstroRenderer } from 'astro/renderer';
import type { SvelteComponent } from 'svelte';

interface SvelteDependencies {
  shared: {},
  server: {
    './SvelteWrapper.svelte': { render: (props: any) => { html: string } }
  },
  client: {
    './runtime.js': typeof import('./runtime')
  }
}

const renderer: AstroRenderer<SvelteDependencies, SvelteComponent> = {
  // To be passed to the Snowpack config
  snowpackPlugin: ['@snowpack/plugin-svelte', { compilerOptions: { hydratable: true } }],

  filter(id) {
    return id.slice(0, -7) === '.svelte';
  },

  server: {
    // Local dependencies work as well
    dependencies: ['./SvelteWrapper.svelte'],
    renderToStaticMarkup({ ["./SvelteWrapper.svelte"]: SvelteWrapper }) {
      return async (Component, props, children) => {
          const { html: code } = SvelteWrapper.render({ __astro_component: Component, __astro_children: children.join('\n'), ...props });
          return { '.html': { code } };
      };
    },
  },

  client: {
    // Local wrapper to provide a JSX-like interface for rendering
    dependencies: ['./runtime.js'],
    hydrateStaticMarkup({ ["./runtime.js"]: runtime }, el) {
      return (Component, props, children) => `
        const {default:render} = ${runtime};
        render(${el}, ${Component}, ${props}, ${children});
      `;
    },
  },
};

export default renderer;

Testing

Tests are passing
Tests updated where necessary

Docs

Docs / READMEs updated
Code comments added where helpful

drwpow · 2021-04-30T16:59:05Z

renderers/preact/index.js

+    jsxFactory: 'h',
+    jsxFragmentFactory: 'Fragment',
+    jsxImportSource: 'preact',
+    validExtensions: ['.jsx', '.tsx'],


One design choice in the Snowpack API was how to assign certain files to certain plugins. We decided on input: [] and output: [] extension arrays as a nicer utility.

Prior art includes webpack’s RegEx-based module loading, as well as Rollup’s resolveId example. We felt that RegEx can get unwieldy very quickly (especially when you’re doing things like negative lookbehinds), but in an ESM world where everything exposes its extensions, we just figured why not use that?

Of course, extensions make it harder when you’re looking for some other part of the module, or for example, you want .css but not .module.css. But we felt making the 95% of usecases simpler outweighed the edge cases (and when in doubt, you could always accept all files of an extension, and return undefined for files you didn’t want to transform).

All that to say, if we want to swap this for a resolveId()-like function, I‘d be game, so long as it stays synchronous because that’s very key for perf.

Thanks for your thoughts! Yeah this is the trickiest part to get right. Don't think I've nailed it yet.

I like the resolveId idea but the specifier alone won't be enough to map a component to a single framework. I think we'll also have to expose the other import specifiers like react or preact.

drwpow · 2021-04-30T17:00:57Z

renderers/preact/index.js

+    jsxImportSource: 'preact',
+    validExtensions: ['.jsx', '.tsx'],
+
+    server: {


Love this separation between server / client! Should this be a function, though, that passes in certain params (I don’t know what exactly; just seems like renderer may need certain context like environment)?

I don't think it needs to be a function. My thought is that renderToStaticMarkup and hydrateStaticMarkup are both passed the necessary context for rendering the component (or hydration script). The wrapper plugin would be instantiated only once, assuming it claims a certain file to render (resolveId or extension matching, etc)

Yeah that’s fair. I just didn’t know if any of the static properties may be affected by environment or not.

drwpow · 2021-04-30T17:03:05Z

renderers/preact/index.js

+export const vue = {
+    jsxFactory: 'h',
+    jsxImportSource: 'vue',
+    validExtensions: ['.vue'],


A really good thing to stress test this design would be Vue + JSX. There’s some wacky stuff going on there (and for an added challenge, try .tsx-flavor of Vue). If you could SSR that, I bet you could SSR anything.

drwpow · 2021-04-30T17:06:26Z

renderers/preact/index.js

+            const { h } = lib;
+            return async (Component, props, children) => {
+                const html = renderToString(h(Component, props, children))
+                return { html }


Another design decision that Snowpack made was having the output be file extensions, e.g.:

{ '.html': { code, map }, '.css': { code, map }, '.json': { code ,map }, }

That way it lets building work with binary files like images & assets as well as UTF8 files without limiting what’s possible. I think we could limit this to only HTML or HTML/CSS if we wanted to; just wanted to ask if there’s ever any chance we’d need another output from this like JSON?

Oh that's a good point. We only need html and css for the current set of frameworks. The output of this file is the server-side representation of this component in the DOM.

But Svelte might need head? And I can imagine some future world where components have a way to declare other types of dependencies.

But Svelte might need head?

Ah good callout. Yeah that might be good to separate the two.

But if we need CSS, then we should probably bake sourcemap support into the return signature (I don’t think that exists for HTML, right?). Just to guilt us into doing it this time 😛

I'm all for it! I like the signature you proposed.

drwpow · 2021-04-30T17:07:33Z

renderers/preact/index.js

+    jsxFragmentFactory: 'Fragment',
+    jsxImportSource: 'react',
+    validExtensions: ['.jsx', '.tsx'],
+    transformChildrenTo: 'jsx',


What does transformChildrenTo do? And why doesn’t preact need it?

Understandable confusion as I haven't implemented it yet! But in order to support passing children, react and preact need to convert their children (HTML strings) back to JSX. vue and svelte support HTML strings, so the other (default) option would be transformChildrenTo: 'string' and we could skip that work.

matthewp · 2021-05-04T14:26:21Z

packages/astro/src/frontend/render/index.ts

+            let prefix = '';
+            let args = []
+            if (clientImports.length === 1) {
+                args[0] = '$a';


What are these about? Can you add a comment explaining?

Yep, just added some code comments!

packages/astro/src/frontend/render/renderer.ts

matthewp · 2021-05-04T14:31:20Z

packages/astro/src/frontend/render/index.ts

+import { childrenToJsx, dedent } from './utils';
+
+export const createRendererPlugin = (plugin: any) => {
+    const { jsxFactory, jsxImportSource, validExtensions, transformChildrenTo } = plugin;


Just a note that we should keep the fact that Astro compiles to h() functions an implementation detail. I realize that this part has to do with converting children into framework nodes and (probably?) doesn't leak the fact that Astro files themselves are h() functions. Let me know if I'm wrong here.

Yes, this is the children => framework vnode utility! Nothing to do with Astro's internal h function. A new name like childrenToFrameworkVNode might be more clear?

matthewp · 2021-05-04T14:37:04Z

Looks good overall. Let's try to avoid making the interface JSX-centric. I think jsxImportSource could be named something different, it's really just a module that the renderer needs to pass through. frameworkImport maybe?

Also is there a reason why rendererSource needs to be imported by Astro and not the plugin?

changeset-bot · 2021-05-12T13:13:05Z

⚠️ No Changeset found

Latest commit: c13e6ff

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

natemoo-re · 2021-05-12T13:43:52Z

@matthewp Great feedback, thanks!

jsxImportSource is literally the source for the jsxFactory and jsxFragmentFactory and could (in theory) be different than frameworkImportSource. Both likely need to be exposed!

I'm working on the Svelte renderer—it won't need to use JSX at all, so that will be a good test to see if this interface is too JSX-centric.

Regarding the rendererSource: keeping everything in Astro seems the most future-proof because we have all the information about deps and can control/optimize when everything runs. I was also concerned that frameworks might get bundled into plugins and get out of sync with the front-end version.

natemoo-re · 2021-05-24T14:34:06Z

Closing in favor of #231 which is infinitely less complex!

natemoo-re self-assigned this Apr 30, 2021

natemoo-re linked an issue Apr 30, 2021 that may be closed by this pull request

RFC: renderer plugins #74

Closed

8 tasks

drwpow reviewed Apr 30, 2021

View reviewed changes

natemoo-re force-pushed the rfc/renderer-plugins branch 2 times, most recently from 1922f86 to e2dcd5f Compare April 30, 2021 21:41

matthewp reviewed May 4, 2021

View reviewed changes

packages/astro/src/frontend/render/renderer.ts Outdated Show resolved Hide resolved

matthewp reviewed May 4, 2021

View reviewed changes

natemoo-re force-pushed the rfc/renderer-plugins branch from ec49df4 to af2ccb8 Compare May 12, 2021 13:13

natemoo-re force-pushed the rfc/renderer-plugins branch from 87105a1 to 2f05f7c Compare May 14, 2021 14:20

Nate Moore added 13 commits May 16, 2021 22:22

wip: renderer plugins

7190071

fix: preact plugin

12fe0db

fix: monorepo issues

3745dd9

chore: move renderers to packages/*

0e4203a

fix: add transfromChildrenTo to preact

8649c8e

docs: add some code comments

30b5623

fix: remove Function.prototype hack

14bcef6

refactor: add new renderer interface

bac15de

chore: update renderer interface, types

160fe88

chore: update Vue renderer type

ec1de16

feat: add svelte renderer

9d6567c

fix: cleanup vue renderer

9ca0fed

refactor: introduce filter to renderers

02aca39

natemoo-re force-pushed the rfc/renderer-plugins branch from 2f05f7c to 02aca39 Compare May 17, 2021 03:22

wip: integrate renderer plugins

c13e6ff

natemoo-re mentioned this pull request May 24, 2021

Renderer plugins #231

Merged

4 tasks

natemoo-re closed this May 24, 2021

natemoo-re deleted the rfc/renderer-plugins branch March 10, 2022 00:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: renderer plugins #155

WIP: renderer plugins #155

natemoo-re commented Apr 30, 2021 •

edited

Loading

drwpow Apr 30, 2021 •

edited

Loading

natemoo-re Apr 30, 2021

drwpow Apr 30, 2021 •

edited

Loading

natemoo-re Apr 30, 2021

drwpow Apr 30, 2021

drwpow Apr 30, 2021

drwpow Apr 30, 2021 •

edited

Loading

natemoo-re Apr 30, 2021

drwpow Apr 30, 2021

natemoo-re Apr 30, 2021

drwpow Apr 30, 2021

natemoo-re Apr 30, 2021

matthewp May 4, 2021

natemoo-re May 12, 2021

matthewp May 4, 2021

natemoo-re May 12, 2021

matthewp commented May 4, 2021

changeset-bot bot commented May 12, 2021 •

edited

Loading

natemoo-re commented May 12, 2021

natemoo-re commented May 24, 2021

WIP: renderer plugins #155

WIP: renderer plugins #155

Conversation

natemoo-re commented Apr 30, 2021 • edited Loading

Todo

Changes

Testing

Docs

drwpow Apr 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drwpow Apr 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drwpow Apr 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewp commented May 4, 2021

changeset-bot bot commented May 12, 2021 • edited Loading

⚠️ No Changeset found

natemoo-re commented May 12, 2021

natemoo-re commented May 24, 2021

natemoo-re commented Apr 30, 2021 •

edited

Loading

drwpow Apr 30, 2021 •

edited

Loading

drwpow Apr 30, 2021 •

edited

Loading

drwpow Apr 30, 2021 •

edited

Loading

changeset-bot bot commented May 12, 2021 •

edited

Loading