Skip to content

feat: add new directives plugin#2070

Open
lxsmnsyc wants to merge 17 commits intomainfrom
feat-directives
Open

feat: add new directives plugin#2070
lxsmnsyc wants to merge 17 commits intomainfrom
feat-directives

Conversation

@lxsmnsyc
Copy link
Member

This PR intends to replace the current directives plugin (TanStack) with our own.

Features:

  • shorter function ids
    • previous plugin used SHA, which produced a very long id. The new one has shorter length (around 8 characters as uid) and has a descriptive name for easier debugging.
  • API change
    • previously we only required createServerReference. This limited the server functions for only top-level declaration. The new plugin now allows inner declarations via cloning (this introduces a new API called cloneServerReference), so now the server functions are able to be registered ahead of time while deferring the actual instance in the exact time it is declared by intention.
    • the previous manifest mechanism is deprecated by server preloading. The plugin keeps track which files has the server functions and then preloads those ahead of time. The function lookup is now done through a map so we don't need to do some dynamic imports. Not sure of the repercussions but there shouldn't be any issues.
  • backwards compatibility
    • should still retain the required features
  • proper treeshaking
    • previous plugin didn't remove some code from a module with a module-level directive. The new one creates an entirely new module from scratch.
  • aliasing
    • the new plugin can now keep track if the server function is exported by alias, so it will be able to retain the export.

@netlify
Copy link

netlify bot commented Feb 19, 2026

Deploy Preview for solid-start-landing-page ready!

Name Link
🔨 Latest commit 21e064d
🔍 Latest deploy log https://app.netlify.com/projects/solid-start-landing-page/deploys/69b66959233c570008a57e7c
😎 Deploy Preview https://deploy-preview-2070--solid-start-landing-page.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@changeset-bot
Copy link

changeset-bot bot commented Feb 19, 2026

⚠️ No Changeset found

Latest commit: 21e064d

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

@pkg-pr-new
Copy link

pkg-pr-new bot commented Feb 19, 2026

Open in StackBlitz

npm i https://pkg.pr.new/solidjs/solid-start/@solidjs/start@2070
npm i https://pkg.pr.new/solidjs/solid-start/@solidjs/vite-plugin-nitro-2@2070

commit: 21e064d

@agoldstein03
Copy link

Hey! Looking to support in any way I can to get this ready, and ideally backported to v1 🙏

This issue has been holding us back from upgrading our very large main codebase past 1.0.10, we have way too much "use server" functionality with transitive imports affected by this, would end up leaking server code to the client without this patch.

I ran a couple tests with this locally on my system and it looks great, there are a couple extra tests I would recommend adding which got hairy with previous hacky implementations I had attempted, and would be nice to have for regression testing:

  • top-level conditional error throwing
    • can be based on isServer for testing (as long as there's no chance of tree shaking identifying that as a tree-shakable const)
      • though more commonly in real code it's based on whether an env var is set or not
    • worth testing both directly in "use server" file and via transitive imports
  • top-level non-function const which calls server code
    • toy example but e.g. import { randomUUID } from "node:crypto"; const GENERATED_ID = randomUUID();
  • checking through production built client code to ensure no server-only consts end up in the client bundle

Happy to write these up and submit them as my best guess to match current test patterns just don't want to contribute anything that might slow down the timeline if the tests don't match expected structure

Anything else I can do please let me know!

@lxsmnsyc
Copy link
Member Author

lxsmnsyc commented Mar 4, 2026

@agoldstein03 Not really sure if it can be backported to 1.0

The two you listed can probably be added, not the third one just because it's tricky to do so

@agoldstein03
Copy link

not the third one just because it's tricky to do so

Linking #2094, related

@yinonburgansky
Copy link
Contributor

  1. When adding a function with an arg to the end of the file
// apps/tests/src/functions/use-is-server-with-anon-default-export.ts

 export default function () {
   return null;
 }
+
+function test(arg: string) {}

It errors with:

$ pnpm --filter tests build
[solid-start:server-functions/compiler] solid-start/apps/tests/use-is-server-with-anon-default-export.ts: Cannot read properties of null (reading 'type')
  1. When trying to test on another project:
// package.json
"@solidjs/start": "https://pkg.pr.new/solidjs/solid-start/@solidjs/start@2070",
"@solidjs/vite-plugin-nitro-2": "https://pkg.pr.new/solidjs/solid-start/@solidjs/vite-plugin-nitro-2@2070",

It errors with:

[vite]: Rollup failed to resolve import "/app/node_modules/.pnpm/@solidjs+start@https+++pkg.pr.new+solidjs+solid-start+@solidjs+start@2070_@testing-libr_073fb0a6dc5a1612314bdc96f37a5371/node_modules/@solidjs/start/dist/server/server-runtime.ts" from "/app/src/f/actions.ts".

@lxsmnsyc
Copy link
Member Author

@yinonburgansky first one should be fixed, not sure with the second one, please try again and see if it persists

@yinonburgansky
Copy link
Contributor

first one should be fixed, not sure with the second one, please try again and see if it persists

Use this git patch on the repo to reproduce:

diff --git a/apps/tests/package.json b/apps/tests/package.json
index 97483d6b..ea52d4fd 100644
--- a/apps/tests/package.json
+++ b/apps/tests/package.json
@@ -16,7 +16,8 @@
   "dependencies": {
     "@solidjs/meta": "^0.29.4",
     "@solidjs/router": "^0.15.3",
-    "@solidjs/start": "workspace:*",
+    "@solidjs/start": "https://pkg.pr.new/solidjs/solid-start/@solidjs/start@2070",
+    "@solidjs/vite-plugin-nitro-2": "https://pkg.pr.new/solidjs/solid-start/@solidjs/vite-plugin-nitro-2@2070",
     "@solidjs/testing-library": "^0.8.10",
     "@testing-library/jest-dom": "^6.6.2",
     "@testing-library/user-event": "^14.5.2",
diff --git a/apps/tests/vite.config.ts b/apps/tests/vite.config.ts
index 4149be66..67bb470e 100644
--- a/apps/tests/vite.config.ts
+++ b/apps/tests/vite.config.ts
@@ -1,6 +1,6 @@
 import { defineConfig } from "vite";
-import { solidStart } from "../../packages/start/src/config";
-import { nitroV2Plugin } from "../../packages/start-nitro-v2-vite-plugin/src";
+import { solidStart } from "@solidjs/start/config"
+import { nitroV2Plugin } from "@solidjs/vite-plugin-nitro-2"
 
 export default defineConfig({
   server: {

apply the patch and run:

$ pnpm dedupe
$ pnpm --filter tests build
error during build:
[vite]: Rollup failed to resolve import "/solid-start/node_modules/.pnpm/@solidjs+start@https+++pkg.pr.new+solidjs+solid-start+@solidjs+start@2070_@testing-libr_38fcfe0e0a57b61247a9db6a54321215/node_modules/@solidjs/start/dist/server/server-runtime.ts" from "/solid-start/apps/tests/src/routes/is-server-nested.tsx?pick=default&pick=$css".

@yinonburgansky
Copy link
Contributor

Another issue is:

$ pnpm --filter tests build
[plugin solid-start:server-functions/compiler] Sourcemap is likely to be incorrect: a plugin (solid-start:server-functions/compiler) was used to transform files, but didn't generate a sourcemap for the transformation. Consult the plugin documentation for help (x27)

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.

3 participants