Adopt tanstack server fn plugin

[brenelz]

PR Checklist

I went deep to try and integrate Tanner's new Server Function plugin.

This was a proof of concept that seems to work.

Need someone more familiar with the code to take a look

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• infrastructure changes
• Other... Please describe:

[changeset-bot]

🦋 Changeset detected

Latest commit: 80c77a5

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

This PR includes changesets to release 1 package

Name	Type
@solidjs/start	Minor

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

[pkg-pr-new]

Open in Stackblitz

More templates

• example-bare
• example-basic
• example-experiments
• example-hackernews
• example-notes
• example-todomvc
• example-with-auth
• example-with-authjs
• example-with-drizzle
• example-with-mdx
• example-with-prisma
• example-with-solid-styled
• example-with-tailwindcss
• example-with-trpc
• example-with-unocss
• example-with-vitest

npm i https://pkg.pr.new/@solidjs/start@1715

commit: 80c77a5

[tannerlinsley]

This is insanely cool.

[birkskyum]

@brenelz , to make the CI green for the docs build, it's just a matter of adjusting the rollup config to avoid the AsyncLocalStorage error for the client bundle.

You can move external

• From: server.rollupConfig.external
• To: vite.build.rollupOptions.external

Or have both for now like below:

// docs/app.config.ts

import { defineConfig } from "@solidjs/start/config";
import tailwindcss from "tailwindcss";
import { config } from "vinxi/plugins/config";

export default defineConfig({
  // experimental: { islands: true },
  server: {
    preset: "cloudflare_module",
    alias: {
      "_mime": "mime/index.js"
    },
    rollupConfig: {
      external: ["__STATIC_CONTENT_MANIFEST", "node:async_hooks"]
    }
  },
  vite: {
    build: {
      rollupOptions: {
        external: ["__STATIC_CONTENT_MANIFEST", "node:async_hooks"]
      }
    },
    plugins: [
      config("tailwind", {
        css: {
          postcss: {
            plugins: [tailwindcss]
          }
        }
      } as any)
    ]
  }
});

[atilafassina]

That's awesome!!
Thanks a lot for this, @brenelz 🏆

Meanwhile I'm working on improving the test story around here so we can merge this more confidently.
Once the CI is green, let's make sure we add a Changeset and then we should be in good shape for reviews / merge. :)

[birkskyum]

Since we don't have a test suite, the manual testing I've done here is to install the pkg.pr.new, and add the below to the Counter of the basic example, and set ssr:false in app config:

function useServer() {
  "use server";  
  console.log("Counter Server Function", isServer) // <- returns True on server
}

export default function Counter() {

  console.log("Counter Init", isServer) // <- returns False in client

  useServer()
  const [count, setCount] = createSignal(0);
  return (
    <button class="increment" onClick={() => setCount(count() + 1)} type="button">
      Clicks: {count()}
    </button>
  );
}

And it appear to work as expected both in vinxi dev mode, and after build + start

[katywings]

I am wondering how easy it would be to test this out in one of my projects via npm link 🙈, or is there a chance we could publish this on some special name/tag to give it a try?

[birkskyum]

@katywings , there is the pkg.pr.new link here - can you use that?

[brenelz]

@birkskyum I was just going to say the same thing. I haven't actually used it before but seems cool

[atilafassina]

Just merged #1719
if we solve the conflicts on this PR we can pull main and it would be more straightforward to add tests.

[birkskyum]

E2E test passing. LGTM

[atilafassina]

I was kinda expecting this one to be a patch since it's not changing any API nor is it adding a new feature.

wdyt?

[birkskyum]

I recommended the minor but I'm good with a patch too.

[katywings]

Testing this out right now, so far it doesn't work at all for me, most likely because it seems to have a completely different approach to decide which dependencies to externalize. Still figuring it out the details though. You gonna get a second analysis soon 😅

Edit: Yeah so it looks like this completely fails to externalize server libraries in the client bundling, resulting in errors during the build. Plus countless warnings for every node built-in module (Module "fs / path / stream / xyz" has been externalized for browser compatibility).

[katywings]

Here is a very basic reproduction of the missing externalization, this works in dev but fails to npm run build.
https://stackblitz.com/edit/github-vts2k1cp?file=src%2Froutes%2Findex.tsx

[katywings]

I wonder if this is a problem in this PR or a general thing with the Tanstack Server Functions plugin 🤔 - afaik @tannerlinsley mentioned that the Tanstack plugin actually should externalize more heavily than Vinxi and not less - so I assume this must be a problem on our side 🤔.

Waiting for this #1722

[birkskyum]

There are externalization tests added to the test suite, so this PR should be rebased, and pass those.

[birkskyum]

@katywings , next time you have a chance to check it, let us know if it fares better for you now with the externalization fixed.