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

feat: refine container APIs for renderers #11251

Merged
merged 1 commit into from
Jun 14, 2024
Merged

Conversation

ematipico
Copy link
Member

@ematipico ematipico commented Jun 13, 2024

Changes

This PR consolidates the API introduced in #11234

Closes #11181

I changed the signature the API to accept an object. The object can accept a renderer (new renderer) and renderer + name for renderers.

Why add name to renderers and making it mandatory? The name comes from the AstroRenderer interface, which is the interface meant to tell Astro how to load the renderer itself. Once the renderer is loaded, Astro adds the name to the resolved renderer. However, with this API, the user must import the renderer itself, and they provide the name themselves. Although the name isn't arbitrary; internally, we have a few exceptions/checks around the name, e.g. @astrojs/lit.

In this PR, I updated the exported server.js files to export the name too, and I added a small interface to make them easily discoverable via TS.

Testing

Updated an existing test

Docs

I updated an existing changeset because we haven't released the former API yet

Copy link

changeset-bot bot commented Jun 13, 2024

⚠️ No Changeset found

Latest commit: b98ba47

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.

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

@github-actions github-actions bot added pkg: svelte Related to Svelte (scope) pkg: vue Related to Vue (scope) pkg: example Related to an example package (scope) pkg: react Related to React (scope) pkg: preact Related to Preact (scope) pkg: solid Related to Solid (scope) pkg: lit Related to Lit (scope) pkg: integration Related to any renderer integration (scope) pkg: astro Related to the core `astro` package (scope) pr: docs A PR that includes documentation for review labels Jun 13, 2024
@ematipico ematipico force-pushed the feat/container-new-api branch from cd11acf to b98ba47 Compare June 13, 2024 15:52
@github-actions github-actions bot removed the pkg: example Related to an example package (scope) label Jun 13, 2024
@matthewp
Copy link
Contributor

I think the fact that renderers have a name is a remnant from before the days we had integrations. So you'd add a renderer directly, and it got a name mostly for debugging purposes.

@ematipico
Copy link
Member Author

Myself and Matt checked that, and unfortunately there are few instances where the name is used for business logics:

  • find the mdx/jsx astro renderer
  • exclude the lit client renderer from doing some stuff

@ematipico ematipico merged commit fd9da98 into main Jun 14, 2024
14 checks passed
@ematipico ematipico deleted the feat/container-new-api branch June 14, 2024 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope) pkg: integration Related to any renderer integration (scope) pkg: lit Related to Lit (scope) pkg: preact Related to Preact (scope) pkg: react Related to React (scope) pkg: solid Related to Solid (scope) pkg: svelte Related to Svelte (scope) pkg: vue Related to Vue (scope) pr: docs A PR that includes documentation for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The Astro got crashed when use the Container API and deploy on vercel with server mode
2 participants