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

💡 RFC: Astro Renderer API update #1487

Closed
1 of 3 tasks
drwpow opened this issue Oct 4, 2021 · 3 comments
Closed
1 of 3 tasks

💡 RFC: Astro Renderer API update #1487

drwpow opened this issue Oct 4, 2021 · 3 comments

Comments

@drwpow
Copy link
Member

drwpow commented Oct 4, 2021

Background & Motivation

Our move from Snowpack to Vite should probably mean our renderer APIs get an update! Currently this is what our renderers contain:

export default {
  external: [],
  polyfills: ['./client-shim.js'],
  hydrationPolyfills: ['lit/experimental-hydrate-support.js'],
  knownEntrypoints: [],
  snowpackPlugins: [] 
};

This interface was designed to work with Snowpack/esinstall (specifically knownEntrypoints).

In a Vite world, this mapping is unclear! And there may be bugs trying to translate things that don’t directly map 1:1. Further, Vite has many more options than Snowpack / esinstall, so we’ll need a more flexible API going forward that allows us to grow with Vite without having to redesign it constantly.

Proposed Solution

I propose taking everything Snowpack-related, and converting it into a viteConfig(config) {} function like so:

import svelte from `@sveltejs/vite-plugin-svelte';

export default {
  // [name, client, server] stay the same
  viteConfig(config) {
    config.ssr.external.push(['…']);
    config.ssr.noExternal.push(['…']);
    config.plugins.push(svelte({ emitCss: true, compilerOptions: { hydratable: true } }))
    return config;
  },
};

By simply exposing the Vite config and allowing each renderer to read it / modify it, it means the renderers can more easily tap into Vite features and plugins without requiring frequent core changes in Astro or renderer API changes.

Alternatives considered

We could expose Vite config keys more directly without a function:

export default {
  ssr: {
    external: [],
    noExternal: [],
  }
}

But I’m -1 for this because a) the SSR API is still beta, and subject to change. And it also makes merging these configs a responsibility of Astro when IMO that should be on the renderer.

Risks, downsides, and/or tradeoffs

One potential downside is that renderers having direct access to the config may mean they do something unexpected, such as interfere with another renderer. But I’d argue that’s already the case; renderers can conflict with one another. But exposing a config function simply removes layers of abstraction that make the conflicts harder to reason about.

Open Questions

In my mind the biggest question is should there be any changes to the JSX plugin? Because Astro allows React and Preact to both handle JSX, I think the JSX renderers will always be “oddballs” that don’t fit the mold. They can probably keep their design for the most part (sidenote: Vite does have first-class React and Preact plugins but they do not work in tandem like Astro allows, so we do need to keep our own).

Detailed Design

No response

Help make it happen!

  • I am willing to submit a PR to implement this change.
  • I am willing to submit a PR to implement this change, but would need some guidance.
  • I am not willing to submit a PR to implement this change.
@matthewp
Copy link
Contributor

RFC Call

  • Async vs sync to be discussed in PR
  • Ready to implement

@okikio
Copy link
Member

okikio commented Oct 12, 2021

Async might be a good option for external configs per renderer or per plugin.

@drwpow
Copy link
Member Author

drwpow commented Nov 4, 2021

This has been completed for 0.21.

@drwpow drwpow closed this as completed Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants