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

use solid-refresh and solid-devtools babel plugins together? #78

Open
tmm1 opened this issue Jan 17, 2025 · 13 comments
Open

use solid-refresh and solid-devtools babel plugins together? #78

tmm1 opened this issue Jan 17, 2025 · 13 comments

Comments

@tmm1
Copy link

tmm1 commented Jan 17, 2025

i'm using @solid-devtools/overlay in a project along with solid-refresh

i noticed in the overlay the props/signals/memos are missing names, which can be solved by injecting solid-devtools/babel's namePlugin into the build pipeline. using this plugin also adds graphs of the reactive relationships between components.

https://github.com/thetarnav/solid-devtools/blob/a9c23a428d5639bf138ac2f5793fc57b8eef9136/packages/main/src/babel/babel.ts#L115-L116

however when I use both the devtools and refresh plugin together, the auto names disappear. are these plugins expected to be able to work together?

i tried putting the devtools namePlugin before and after the solid-refresh one.

@tmm1
Copy link
Author

tmm1 commented Jan 17, 2025

with solid-devtools only:

Image

with solid-refresh:

Image Image Image

@tmm1
Copy link
Author

tmm1 commented Jan 17, 2025

Looking at the generated code, this is probably a bug on the devtools side.

I see some components are annotated:

const SidebarMenuItem = _$$component(_REGISTRY, "SidebarMenuItem", function SidebarMenuItem(props: {
}) {
  const iconClass = createMemo(() => {
...
  }, undefined, {
    name: "iconClass"
  });

whereas others are not:

const SidebarMenu = _$$component(_REGISTRY, "SidebarMenu", function SidebarMenu(props: {
}) {
  const showedPinned = createMemo(() => {
...
  });

@tmm1
Copy link
Author

tmm1 commented Jan 17, 2025

Okay confirmed this is a solid-devtools bug. The workaround is:

-export function SidebarMenu(
+export { SidebarMenu };
+function SidebarMenu(

@tmm1
Copy link
Author

tmm1 commented Jan 17, 2025

actually, solid-devtools works as-is with both types of components.. the issue is some kind of interaction with solid-refresh

if i put the devtools babel plugin first, nothing in the output is annotated.

if i put the devtools plugin after solid-refresh, then i get partial annotation which makes sense since devtools isn't setup for the refresh AST?

the correct configuration would seem to be devtools first so everything is annotated, then solid-refresh should be able to understand and maintain those annotations? currently it seems to remove them all?

@tmm1 tmm1 reopened this Jan 17, 2025
@tmm1
Copy link
Author

tmm1 commented Jan 17, 2025

well i tried a clean build with devtools plugin first and solid-refresh second and everything seems to be working fine.

EDIT: scratch that, still had my workaround in place

@tmm1 tmm1 closed this as completed Jan 17, 2025
@tmm1 tmm1 reopened this Jan 17, 2025
@tmm1
Copy link
Author

tmm1 commented Jan 17, 2025

as far as I can tell, somehow when bubbleFunctionDeclaration moves a export function Component, somehow the non-exported node it adds back in is a copy from earlier in the babel pipeline.

@tmm1
Copy link
Author

tmm1 commented Jan 17, 2025

theory from o1:

halluciation

Root cause explanation

From the code and transcript, here is what is actually going on:

  1. You have an exported function component, for example:

    export function SidebarMenu() {
      const [sidebarWidth, setSidebarWidth] = createSignal<number>(0, {
        name: "sidebarWidth"
      });
      
    }
  2. The “solid-devtools” plugin injects or preserves the second argument object (the metadata) in the call to createSignal (e.g. “{ name: "sidebarWidth" }”).

  3. However, when “solid-refresh” wraps the function (inside transformFunctionDeclaration + wrapComponent), Babel re-creates or re-serializes the function’s AST. The key detail is that the “<number>” type parameter (a TSCallType parameter) often ends up ignored or stripped at some point (for example, if a piece of the plugin logic tries to reinterpret the call node without preserving its type parameters). As a result:

    • The AST’s call arguments effectively shift.
    • Babel ends up treating the object literal (“{ name: "sidebarWidth" }”) as if it was never there, or fabricates a single argument call (just “(0)”).

Hence the second argument with your metadata object goes missing in the final output. The simplest way to think about it is: the plugin does not fully preserve TS type parameters on function calls like createSignal, and in dropping or ignoring type information, the second argument array element winds up lost.

In other words, the bug is not in moving or re‑defining the function as a variable. It’s that the type parameter () is confusing (or being stripped by) the refresh code’s AST transforms, which causes the entire extra argument—your annotation object—to get lost when the final output is generated.

@lxsmnsyc
Copy link
Member

btw how are you setting up the plugins

@tmm1
Copy link
Author

tmm1 commented Jan 17, 2025

one weird thing i discovered is if i left the original component in place, then both copies would be correct.

root cause here seems to be related to how babel runs plugins. it does one pass with all of the plugins visitors running once?

the simplest fix i found so far is to move the function defintions to the bottom instead of top, which seems to ensure they get processed correctly.

const last = program.get('body').at(-1);
const [tmp] = last.insertAfter(decl);

EDIT: tht doesn't work either, just results in ReferenceError: Cannot access 'xxx' before initialization

@tmm1
Copy link
Author

tmm1 commented Jan 17, 2025

btw how are you setting up the plugins

["module:solid-devtools/babel/named"],
["module:solid-refresh/babel", { bundler: "vite" }],

with patch-package and a patches/solid-devtools+0.33.0.patch

patch file
diff --git a/node_modules/solid-devtools/dist/babel/named.js b/node_modules/solid-devtools/dist/babel/named.js
new file mode 100644
index 0000000..dc3e20c
--- /dev/null
+++ b/node_modules/solid-devtools/dist/babel/named.js
@@ -0,0 +1,5 @@
+import {
+  namePlugin
+} from "../chunk-RDZMZMK7.js";
+export default function() { return namePlugin };
+
diff --git a/node_modules/solid-devtools/package.json b/node_modules/solid-devtools/package.json
index 6e3d53a..73cda29 100644
--- a/node_modules/solid-devtools/package.json
+++ b/node_modules/solid-devtools/package.json
@@ -78,6 +78,11 @@
         "default": "./dist/babel.js"
       }
     },
+    "./babel/named": {
+      "import": {
+        "default": "./dist/babel/named.js"
+      }
+    },
     "./package.json": "./package.json"
   },
   "typesVersions": {

see thetarnav/solid-devtools#295 (comment)

i think probably this would work:

import { namePlugin } from 'solid-devtools/babel'

...

plugins: [
    namePlugin,
    ["module:solid-refresh/babel"]

@tmm1
Copy link
Author

tmm1 commented Jan 17, 2025

babel/babel#5854

http://thejameskyle.com/babel-plugin-ordering.html

basically if we remove the node while solid-devtools plugin is concurrently getting around to processing it, it never gets modified so the moved copy at the top of the file is the original AST.

babel/babel#15265 (comment)

babel/babel#11147 (comment)

@tmm1
Copy link
Author

tmm1 commented Jan 18, 2025

http://thejameskyle.com/babel-plugin-ordering.html

this post describes the underlying issue well. both plugins use a Program visitor so ordering is problematic.

i tried to wrangle babel to make this work, but in the end i had to do it manually.

--- a/node_modules/solid-refresh/dist/babel.mjs
+++ b/node_modules/solid-refresh/dist/babel.mjs
@@ -1,6 +1,7 @@
 import * as t from '@babel/types';
 import path from 'path';
 import _generator from '@babel/generator';
+import { namePlugin } from 'solid-devtools/babel';

 // This is just a Pascal heuristic
 // we only assume a function is a component
@@ -1008,6 +1009,9 @@ function solidRefreshPlugin() {
                 if (setupProgram(state, programPath, context.file.ast.comments)) {
                     return;
                 }
+                const devtools = namePlugin.visitor
+                programPath._call(devtools.Program.enter)
+                programPath.traverse(devtools);
                 programPath.traverse({
                     FunctionDeclaration(path) {
                         bubbleFunctionDeclaration(programPath, path);

@lxsmnsyc
Copy link
Member

that's not exactly reliable, but I can't blame you for that.

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 a pull request may close this issue.

2 participants