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

Expose JSX compilation to renderers #588

Merged
merged 17 commits into from
Jul 21, 2021
Merged

Expose JSX compilation to renderers #588

merged 17 commits into from
Jul 21, 2021

Conversation

natemoo-re
Copy link
Member

@natemoo-re natemoo-re commented Jun 30, 2021

This is slated for astro@0.18.0!

Terminology

  • "JSX" is used here to refer to both .jsx and .tsx files. They are conceptually interchangeable.

Motivation

As it turns out, JSX is really complicated! Most JSX approaches (rightly) assume that your entire project is full of components that should be compiled the same way. Astro is in a unique position by promising that users can mix components from different frameworks together—what is simple for React + Vue + Svelte becomes very complicated for JSX-based frameworks like React + Preact + Solid.

The only reason Astro currently allows React + Preact is because Snowpack smartly detects import { h } from 'preact', defaulting to React otherwise. We can't scale this approach.

At the same time, JSX is a common syntax without universal agreement on how compilation should work. The most common approach is to use jsxFactory (React.createElement or h) and jsxFragment (React.Fragment or Fragment), and this is all esbuild supports. Frameworks are moving away from this approach and tools are moving to support these more complex JSX configurations.

React recently introduced a new JSX Transform. Vue takes yet another approach. Solid compiles JSX without any VDOM implementation (similar to Svelte) and expects different output in SSR mode than on the client.

Goals

  • Find a way for renderers to take granularly take responsibility for a JSX file, to enable per-file JSX compilation.
  • Support non-standard JSX compilation that does not use a simple global pragma.
  • Support JSX compilation that should behave differently for SSR versus client
  • Stretch Users expect to not import { jsxPragma } from 'lib'. Can we enable this for convenience?
  • Stretch While we're taking responsibility for compiling JSX, can we automatically enable tools like react-refresh and prefresh?

Solution

At a high-level, this PR does a few things:

  • Introduces a Snowpack Plugin to load .jsx and .tsx files.
  • Allows renderers to manage JSX compilation on a per-file basis. es-module-lexer scans the loaded file for imports and matches that to the renderer. Files with no imports may use a /** @jsxImportSource my-library */ comment.
  • Allows renderers to compile JSX files. Renderers can control how a JSX file is transformed and are passed the load context (isSSR, isDev, isHmrEnabled, etc). Babel is the de facto solution for this—every framework has a Babel plugin. Therefore, renderers may configure which Babel plugins should be used to process the JSX file.

Problems, Tradeoffs

@babel/core is slow. We should use esbuild!

esbuild is awesome, but it only supports jsxFactory and jsxFragment. Vite enables automatic imports with the work-around jsxInject option, which Snowpack introduced as well. This doesn't solve the core problem here, which is that not every JSX file uses jsxFactory!

Every framework supports Babel. In practice, every JSX file in Snowpack is likely being run through Babel already! @snowpack/plugin-react-refresh and @prefresh/snowpack both use it for transform. Any users of other frameworks are likely using a custom Babel config and relying on @snowpack/plugin-babel.

This PR also attempts to mitigate Babel's slowness as much as possible. esbuild pre-compiles the file, transforming most non-standard syntax and removing TypeScript—the only non-standard syntax preserved is JSX. So Babel's only responsibility here is to compile JSX to normal JS, which should be relatively fast.

There is precedent for this in the linked Snowpack plugins above, and those haven't been a particular performance concern. Babel can be fast if you keep it focused.

Multi-framework support is an edge case. Multi-JSX-framework support is and edge case within that edge case.

Multi-framework is probably a 20% use case feature of Astro, but it is a very important conceptual one. This PR makes every attempt to skip work when only a single renderer is enabled, which we can guess is about 80% of the time. This PR's more complex scanning behavior will kick-in only if multiple JSX-supporting renderers are enabled.

At the same time, this PR actually makes life easier for users of a single framework! They won't need to configure any JSX compilation using snowpack.config.mjs or manually import their JSX factory function. Again, Preact is special-cased in Snowpack so we're not seeing that pain for users of smaller frameworks. The JSX factory import has tripped up quite a few users coming from Next.js (Jason, Cassidy, etc).

Finally, mixing multiple JSX frameworks shouldn't be any more difficult than mixing React + Vue + Svelte! It's the same feature. Astro should own that complexity, not the user.

Imagine how great it would be for React users to test out Solid on a page or two without any extra hassle. Just add @astrojs/renderer-solid and you're good to go.

We shouldn't deal with Fast Refresh here!

Maybe, but shouldn't we if it's a small lift? It's clear that HMR is table stakes for dev tooling, but Fast Refresh (aka stateful HMR) is increasingly moving that way, too. The "fast refresh" plugins for Snowpack are just running a file through Babel anyway, so this would remove the need for those and make users super happy.

Testing

  • TODO: add tests

Docs

  • TODO: add docs

@vercel
Copy link

vercel bot commented Jun 30, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

astro-docs – ./docs

🔍 Inspect: https://vercel.com/pikapkg/astro-docs/FzNbhn1j6asC2VSVVUjERGFus8uC
✅ Preview: https://astro-docs-git-feat-improve-jsx-pikapkg.vercel.app

astro-www – ./www

🔍 Inspect: https://vercel.com/pikapkg/astro-www/3sfrHMiP1TruvUiV2aGKEZtgvWui
✅ Preview: https://astro-www-git-feat-improve-jsx-pikapkg.vercel.app

@changeset-bot
Copy link

changeset-bot bot commented Jun 30, 2021

🦋 Changeset detected

Latest commit: 1602ab5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@astrojs/renderer-solid Minor
@astrojs/renderer-preact Minor
@astrojs/renderer-react Minor
create-astro Patch
astro Patch

Not sure what this means? Click here to learn what changesets are.

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

@matthewp
Copy link
Contributor

Before you go too far with this, this seems like a heavy solution. I don't think we want to add babel processing to every .jsx file if we can avoid it. This also runs esbuild multiple times, I'm sure there's a reason but that is going to be a performance issue.

Can we not just prepend the appropriate import statement to the file if its missing?

@natemoo-re
Copy link
Member Author

@matthewp You're right that this is a pretty heavy solution, but there are very good reasons for handling it this way! I'll take some time to write up a good outline before I write any more code. It should make more sense in context.

That being said, the double esbuild transform is a hack and I'm planning to remove it soon.

@natemoo-re natemoo-re changed the title Add support for jsx-runtime, allow renderers to automate JSX Expose control of compiling JSX to renderers Jul 1, 2021
@natemoo-re natemoo-re changed the title Expose control of compiling JSX to renderers Expose JSX compilation to renderers Jul 1, 2021
@natemoo-re
Copy link
Member Author

cc @eyelidlessness, I know this was one blocker for the Solid renderer.

@jasikpark
Copy link
Contributor

Would SWC be a viable way to pull in babel transforms? https://swc.rs/docs/comparison-babel/ -- it's written in Rust, so it should be fast similar to esbuild...

@natemoo-re
Copy link
Member Author

natemoo-re commented Jul 6, 2021

Was able to build 98% of a Solid renderer based on this branch, which was the main motivation here. Check out the diff. There seems to be a hydration issue to work out, might need to connect with their team on that.

Specifically check out the added jsxTransformOptions setting which controls exactly how babel should run.

@natemoo-re
Copy link
Member Author

Would SWC be a viable way to pull in babel transforms? https://swc.rs/docs/comparison-babel/ -- it's written in Rust, so it should be fast similar to esbuild...

Ahh. Correct me if I'm wrong, but I think that link just compares SWC's featureset to the most common babel plugins. It doesn't look like SWC can actually run babel plugins.

@jasikpark
Copy link
Contributor

I suppose you're right, since otherwise that would probably be listed in https://swc.rs/docs/usage-plugin rather than describing only how to make swc specific plugins.

@jasikpark
Copy link
Contributor

jasikpark commented Jul 7, 2021

swc-project/swc#1465 It does seem like they are working on babel plugin support though 🤔 I'll look into it more.
aha: yes it is the plan for babel plugins to be supported swc-project/swc#1615

Nate Moore and others added 15 commits July 21, 2021 12:40
* feat: add support for `jsxImportSource`, new JSX transform

* WIP: solid renderer

* [Renderer] Solid (#656)

* feat: add support for `jsxImportSource`, new JSX transform

* WIP: solid renderer

* Solid renderer: fix SSR of children, hydration (top level)

Caveat: cannot hydrate children/descendants of hydrated parents

* Fix hydration of fragments

* fix: SyntaxError in React/Preact renderers

* fix: errors in React/Preact renderers

* feat: update react external

* chore: update examples

* chore: delete old changelog

* chore: update astro config

Co-authored-by: Nate Moore <nate@skypack.dev>

* Changing the preact to Solid (#669)

* chore: use new client:visible syntax

* fix: dev script issue

* chore: cleanup SolidJS example

* docs: update framework example docs

* chore: cleanup framework-multiple example

* fix: remove SolidJS false-positives from Preact renderer

* chore: add changeset

Co-authored-by: eyelidlessness <eyelidlessness@users.noreply.github.com>
Co-authored-by: Abdullah Mzaien <s201540830@kfupm.edu.sa>
@natemoo-re
Copy link
Member Author

Tests should pass once FredKSchott/snowpack#3602 lands and gets merged back in

Copy link
Member

@FredKSchott FredKSchott left a comment

Choose a reason for hiding this comment

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

LGTM once the tests pass!

@natemoo-re natemoo-re merged commit bd18e14 into main Jul 21, 2021
@natemoo-re natemoo-re deleted the feat/improve-jsx branch July 21, 2021 23:10
FredKSchott pushed a commit that referenced this pull request Jul 22, 2021
* feat: add support for `jsxImportSource`, new JSX transform

* Renderer: add Solid renderer (#667)

* feat: add support for `jsxImportSource`, new JSX transform

* WIP: solid renderer

* [Renderer] Solid (#656)

* feat: add support for `jsxImportSource`, new JSX transform

* WIP: solid renderer

* Solid renderer: fix SSR of children, hydration (top level)

Caveat: cannot hydrate children/descendants of hydrated parents

* Fix hydration of fragments

* fix: SyntaxError in React/Preact renderers

* fix: errors in React/Preact renderers

* feat: update react external

* chore: update examples

* chore: delete old changelog

* chore: update astro config

Co-authored-by: Nate Moore <nate@skypack.dev>

* Changing the preact to Solid (#669)

* chore: use new client:visible syntax

* fix: dev script issue

* chore: cleanup SolidJS example

* docs: update framework example docs

* chore: cleanup framework-multiple example

* fix: remove SolidJS false-positives from Preact renderer

* chore: add changeset

Co-authored-by: eyelidlessness <eyelidlessness@users.noreply.github.com>
Co-authored-by: Abdullah Mzaien <s201540830@kfupm.edu.sa>

* feat(create-astro): add Solid support

* docs: add JSX options to renderer reference

* chore: add changeset for P/React renderers

* fix: move react/server.js to external

* chore: remove brewfile

* Revert "feat: add support for `jsxImportSource`, new JSX transform"

This reverts commit 077c4bf.

* fix: remove `react-dom/server` from `external`

* chore: remove unused dependency

* feat: improve JSX error messages

* Revert "Revert "feat: add support for `jsxImportSource`, new JSX transform""

This reverts commit f6c2896.

* docs: update jsxImportSource

* feat: improve error message

* feat: improve error logging for JSX renderers

* tests: add jsx-runtime tests

* chore: update snowpack

Co-authored-by: eyelidlessness <eyelidlessness@users.noreply.github.com>
Co-authored-by: Abdullah Mzaien <s201540830@kfupm.edu.sa>
SiriousHunter pushed a commit to SiriousHunter/astro that referenced this pull request Feb 3, 2023
* feat: add support for `jsxImportSource`, new JSX transform

* Renderer: add Solid renderer (withastro#667)

* feat: add support for `jsxImportSource`, new JSX transform

* WIP: solid renderer

* [Renderer] Solid (withastro#656)

* feat: add support for `jsxImportSource`, new JSX transform

* WIP: solid renderer

* Solid renderer: fix SSR of children, hydration (top level)

Caveat: cannot hydrate children/descendants of hydrated parents

* Fix hydration of fragments

* fix: SyntaxError in React/Preact renderers

* fix: errors in React/Preact renderers

* feat: update react external

* chore: update examples

* chore: delete old changelog

* chore: update astro config

Co-authored-by: Nate Moore <nate@skypack.dev>

* Changing the preact to Solid (withastro#669)

* chore: use new client:visible syntax

* fix: dev script issue

* chore: cleanup SolidJS example

* docs: update framework example docs

* chore: cleanup framework-multiple example

* fix: remove SolidJS false-positives from Preact renderer

* chore: add changeset

Co-authored-by: eyelidlessness <eyelidlessness@users.noreply.github.com>
Co-authored-by: Abdullah Mzaien <s201540830@kfupm.edu.sa>

* feat(create-astro): add Solid support

* docs: add JSX options to renderer reference

* chore: add changeset for P/React renderers

* fix: move react/server.js to external

* chore: remove brewfile

* Revert "feat: add support for `jsxImportSource`, new JSX transform"

This reverts commit 077c4bf.

* fix: remove `react-dom/server` from `external`

* chore: remove unused dependency

* feat: improve JSX error messages

* Revert "Revert "feat: add support for `jsxImportSource`, new JSX transform""

This reverts commit f6c2896.

* docs: update jsxImportSource

* feat: improve error message

* feat: improve error logging for JSX renderers

* tests: add jsx-runtime tests

* chore: update snowpack

Co-authored-by: eyelidlessness <eyelidlessness@users.noreply.github.com>
Co-authored-by: Abdullah Mzaien <s201540830@kfupm.edu.sa>
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

Successfully merging this pull request may close these issues.

4 participants