From 4e378842e41179a29cd4de9ecf6d63b6b6bbed45 Mon Sep 17 00:00:00 2001 From: Max Amundsen Date: Wed, 15 Jul 2026 11:28:24 -0400 Subject: [PATCH] update solid compiler --- go/jsbundler/compile_solid_gen.go | 111 ++++++++++++++++++---- go/jsbundler/compile_solid_render_test.go | 71 ++++++++++++++ go/jsbundler/hmr_e2e_test.go | 18 ++-- go/jsbundler/hmr_server.go | 11 +++ go/jsbundler/hmr_watch.go | 16 ++++ go/jsbundler/js.go | 10 +- go/tw/tailwind.go | 6 +- go/validation/validation.go | 50 ++++++++++ 8 files changed, 255 insertions(+), 38 deletions(-) diff --git a/go/jsbundler/compile_solid_gen.go b/go/jsbundler/compile_solid_gen.go index c66b8a0e..aaaf60ec 100644 --- a/go/jsbundler/compile_solid_gen.go +++ b/go/jsbundler/compile_solid_gen.go @@ -4,8 +4,9 @@ package jsbundler // // Strategy (behavior-parity with babel-preset-solid, not byte-parity): // - Static element structure is baked into an HTML template string cloned at -// runtime via _$template(); we always quote attrs and emit closing tags -// (valid HTML the browser parses to the same DOM babel's terser template does). +// runtime via _$template(); we always quote attrs and emit closing tags, +// except for void elements (br, img, input, ...) which never get one — +// see voidElements. // - Dynamic children -> _$insert(parent, () => expr, marker). Every dynamic // expression is wrapped in a thunk: correct and (for static exprs) merely an // extra no-op effect. babel unwraps `f()` -> `f` as an optimization; we skip @@ -461,11 +462,16 @@ func (g *solidGen) genElement(node *jsxNode) string { return tv + "()" } var b strings.Builder - b.WriteString("(() => { var " + strings.Join(c.decls, ", ") + "; ") + b.WriteString("(() => { var ") + b.WriteString(strings.Join(c.decls, ", ")) + b.WriteString("; ") for _, op := range c.ops { - b.WriteString(op + "; ") + b.WriteString(op) + b.WriteString("; ") } - b.WriteString("return " + root + "; })()") + b.WriteString("return ") + b.WriteString(root) + b.WriteString("; })()") return b.String() } @@ -566,7 +572,7 @@ func exprIsEmpty(expr string) bool { func (c *iife) assignChildRefs(node *jsxNode, parentRef string) map[int]string { refs := map[int]string{} // Which children need a ref? - need := func(i int, ch *jsxNode) bool { + need := func(_ int, ch *jsxNode) bool { switch ch.kind { case jsxElement: return elementNeedsRef(ch) @@ -644,6 +650,14 @@ func (c *iife) attr(ref string, a jsxAttr) { // innerHTML icon content would never appear). Assign the property. c.ops = append(c.ops, fmt.Sprintf("%s(() => %s.%s = %s)", c.g.helper("effect"), ref, a.name, expr)) + case booleanAttrs[a.name]: + // HTML boolean attributes are presence-based: setAttribute(name, + // String(value)) would emit `checked="false"`, which the browser + // parses as present (any string value) -> checked stays true. Use + // _$setBoolAttribute, which adds the attribute (empty) when truthy + // and removes it when falsy. + c.ops = append(c.ops, fmt.Sprintf("%s(() => %s(%s, %q, %s))", + c.g.helper("effect"), c.g.helper("setBoolAttribute"), ref, a.name, expr)) default: c.ops = append(c.ops, fmt.Sprintf("%s(() => %s(%s, %q, %s))", c.g.helper("effect"), c.g.helper("setAttribute"), ref, a.name, expr)) @@ -725,30 +739,44 @@ func (g *solidGen) genComponent(node *jsxNode) string { } // componentProps builds the props object: static attrs as plain values, dynamic -// attrs as reactive getters, and children as a `children` prop. +// attrs as reactive getters, children as a `children` prop, and any {...spread} +// threaded through the mergeProps helper (mirroring spread(), preserving source +// order so later props override earlier ones, matching babel). func (g *solidGen) componentProps(node *jsxNode) string { - var parts []string + var args []string + var obj []string + flush := func() { + if len(obj) > 0 { + args = append(args, "{ "+strings.Join(obj, ", ")+" }") + obj = nil + } + } for _, a := range node.attrs { switch a.kind { case attrStatic: if a.boolt { - parts = append(parts, fmt.Sprintf("%s: true", propKey(a.name))) + obj = append(obj, fmt.Sprintf("%s: true", propKey(a.name))) } else { - parts = append(parts, fmt.Sprintf("%s: %s", propKey(a.name), jsStringLit(a.value))) + obj = append(obj, fmt.Sprintf("%s: %s", propKey(a.name), jsStringLit(a.value))) } case attrExpr: - parts = append(parts, fmt.Sprintf("get %s() { return %s; }", propKey(a.name), g.compileExpr(a.expr))) + obj = append(obj, fmt.Sprintf("get %s() { return %s; }", propKey(a.name), g.compileExpr(a.expr))) case attrSpread: - // milestone 3 (mergeProps) + flush() + args = append(args, g.compileExpr(a.expr)) } } if ch := g.childrenProp(node); ch != "" { - parts = append(parts, ch) + obj = append(obj, ch) } - if len(parts) == 0 { + flush() + if len(args) == 0 { return "{}" } - return "{ " + strings.Join(parts, ", ") + " }" + if len(args) == 1 { + return args[0] + } + return fmt.Sprintf("%s(%s)", g.helper("mergeProps"), strings.Join(args, ", ")) } // childrenProp builds a component's `children` prop entry. JSX children are @@ -786,8 +814,8 @@ func (g *solidGen) genFragment(node *jsxNode) string { var kids []*jsxNode for i := range node.children { ch := &node.children[i] - if ch.kind == jsxText && strings.TrimSpace(ch.text) == "" { - continue + if !renderedChild(ch) { + continue // drop whitespace-only text and comment-only expressions ({/* ... */}) } kids = append(kids, ch) } @@ -807,6 +835,21 @@ func (g *solidGen) genFragment(node *jsxNode) string { // ---- template building ----------------------------------------------------- +// voidElements are HTML elements that can never have a closing tag or +// children. Writing one out as `` isn't just redundant — for `br` +// specifically it's actively wrong: the HTML parser doesn't ignore a stray +// `
` like it does for other unmatched end tags, it's spec'd to act as +// *another* `
` start tag. That silently inserts an extra DOM node, +// shifting every subsequent .firstChild/.nextSibling in the compiled +// navigation chain by one and crashing template hydration. Matches the list +// solid-js/html's own VOID_ELEMENTS uses (frontend/vendor/solid-js/html). +var voidElements = map[string]bool{ + "area": true, "base": true, "br": true, "col": true, "embed": true, + "hr": true, "img": true, "input": true, "keygen": true, "link": true, + "menuitem": true, "meta": true, "param": true, "source": true, + "track": true, "wbr": true, +} + func buildTemplate(node *jsxNode) string { var sb strings.Builder writeTemplate(node, &sb) @@ -814,19 +857,28 @@ func buildTemplate(node *jsxNode) string { } func writeTemplate(node *jsxNode, sb *strings.Builder) { - sb.WriteString("<" + node.tag) + sb.WriteString("<") + sb.WriteString(node.tag) if !hasSpread(node) { // with a spread, all attrs are applied at runtime for _, a := range node.attrs { if a.kind == attrStatic { if a.boolt { - sb.WriteString(" " + a.name) + sb.WriteString(" ") + sb.WriteString(a.name) } else { - sb.WriteString(" " + a.name + `="` + a.value + `"`) + sb.WriteString(" ") + sb.WriteString(a.name) + sb.WriteString(`="`) + sb.WriteString(a.value) + sb.WriteString(`"`) } } } } sb.WriteString(">") + if voidElements[strings.ToLower(node.tag)] { + return // no children, no closing tag + } for i := range node.children { ch := &node.children[i] switch ch.kind { @@ -840,7 +892,9 @@ func writeTemplate(node *jsxNode, sb *strings.Builder) { } } } - sb.WriteString("") + sb.WriteString("") } // annotateMarkers sets node.marker on dynamic children that need a `` anchor: @@ -904,6 +958,21 @@ func isStaticLiteral(e string) bool { // direct property assignment rather than setAttribute. var contentProps = map[string]bool{"innerHTML": true, "textContent": true, "innerText": true} +// booleanAttrs are HTML boolean attributes: presence means true regardless of +// the attribute's string value, so a naive setAttribute(name, String(false)) +// (which emits e.g. checked="false") leaves the attribute present and the +// browser reads it as true. These must go through setBoolAttribute instead, +// which adds/removes the attribute based on the JS value's truthiness. +var booleanAttrs = map[string]bool{ + "allowfullscreen": true, "async": true, "autofocus": true, "autoplay": true, + "checked": true, "controls": true, "default": true, "defer": true, + "disabled": true, "formnovalidate": true, "hidden": true, "indeterminate": true, + "ismap": true, "loop": true, "multiple": true, "muted": true, + "nomodule": true, "novalidate": true, "open": true, "playsinline": true, + "readonly": true, "required": true, "reversed": true, "seamless": true, + "selected": true, +} + func isEventAttr(name string) bool { // An event handler is `on` followed by the event name; casing after `on` is // irrelevant (onClick and onclick both mean click) — the name is lowercased diff --git a/go/jsbundler/compile_solid_render_test.go b/go/jsbundler/compile_solid_render_test.go index bb911c5b..c61cd508 100644 --- a/go/jsbundler/compile_solid_render_test.go +++ b/go/jsbundler/compile_solid_render_test.go @@ -151,6 +151,9 @@ func TestGoCompilerRenderSpreadRef(t *testing.T) { {"spread", `export const A = () => { const p = { id: "pid", title: "t" }; return
hi
; };`}, {"spread-override", `export const A = () => { const p = { class: "from-p" }; return
hi
; };`}, {"ref", `export const A = () => { let r; return
hi
; };`}, + {"component-spread", `export const A = () => { const Box = (props) =>
{props.children}
; const p = { id: "pid", title: "t" }; return hi; };`}, + {"component-spread-override", `export const A = () => { const Box = (props) =>
x
; const p = { class: "from-p" }; return ; };`}, + {"component-spread-before", `export const A = () => { const Box = (props) =>
x
; const p = { class: "from-p" }; return ; };`}, } for _, c := range cases { c := c @@ -299,3 +302,71 @@ func TestGoCompilerStyleAndInnerHTML(t *testing.T) { }) } } + +// Regression: HTML boolean attributes (checked, disabled, required, readonly, +// hidden, selected, ...) are presence-based — any attribute value, including +// the string "false", still counts as present. A dynamic false must go through +// setBoolAttribute (which removes the attribute), not setAttribute (which would +// emit e.g. checked="false", read by the browser as checked=true). +func TestGoCompilerBooleanAttrs(t *testing.T) { + root, _ := filepath.Abs("../..") + t.Chdir(root) + src := `export const A = () => { + const no = () => false; + const yes = () => true; + return
+ + +
; + };` + out, err := compileSolidGo(src, "bools.tsx", false) + if err != nil { + t.Fatal(err) + } + html, err := renderComponent(t, out) + if err != nil { + t.Fatalf("render: %v\n%s", err, out) + } + for _, broken := range []string{`checked="false"`, `disabled="false"`} { + if strings.Contains(html, broken) { + t.Errorf("false-valued boolean attr still present (reads as true in browser): %q in %s", broken, html) + } + } + for _, present := range []string{`checked=""`, `required=""`} { + if !strings.Contains(html, present) { + t.Errorf("true-valued boolean attr missing: expected %q in %s", present, html) + } + } +} + +// Regression: a comment-only JSX expression child ({/* note */}) must be +// dropped like whitespace, not compiled as a real expression. childrenProp +// (component children) already filtered these via renderedChild, but a bare +// root fragment with 2+ children went through genFragment's own filter, which +// only skipped blank text — a comment-only child slipped through and compiled +// to `_$memo(() => /* note */)`, a syntax error that breaks the whole module. +func TestGoCompilerFragmentCommentChild(t *testing.T) { + root, _ := filepath.Abs("../..") + t.Chdir(root) + src := `export const A = () => { + return <> + {/* a comment */} +
one
+
two
+ ; + };` + out, err := compileSolidGo(src, "frag-comment.tsx", false) + if err != nil { + t.Fatal(err) + } + if err := validateJS(out); err != nil { + t.Fatalf("compiled output doesn't parse: %v\n--- output ---\n%s", err, out) + } + html, err := renderComponent(t, out) + if err != nil { + t.Fatalf("render: %v\n%s", err, out) + } + if !strings.Contains(html, "one") || !strings.Contains(html, "two") { + t.Errorf("expected both siblings in output, got: %q", html) + } +} diff --git a/go/jsbundler/hmr_e2e_test.go b/go/jsbundler/hmr_e2e_test.go index a0a7d9e2..1055e441 100644 --- a/go/jsbundler/hmr_e2e_test.go +++ b/go/jsbundler/hmr_e2e_test.go @@ -115,20 +115,20 @@ func TestDevServerEndToEnd(t *testing.T) { } // --- SPA entry: hot bootstrap + bare imports kept + relative imports rewritten - code, app := get(srcURLPrefix + "app.ts") + code, app := get(srcURLPrefix + "app.tsx") if code != 200 { - t.Fatalf("app.ts status %d:\n%s", code, app) + t.Fatalf("app.tsx status %d:\n%s", code, app) } for _, want := range []string{ - `__createHotContext("/@src/app.ts")`, // hot bootstrap - `/@hmr/client`, // client import injected - `"solid-js/web"`, // bare specifier preserved for import map - `"@solidjs/router"`, // bare specifier preserved - `/@src/routes/app-routes.ts`, // relative import rewritten - `/@src/layouts/AppLayout.ts`, // relative import rewritten + `__createHotContext("/@src/app.tsx")`, // hot bootstrap + `/@hmr/client`, // client import injected + `"solid-js/web"`, // bare specifier preserved for import map + `"@solidjs/router"`, // bare specifier preserved + `/@src/routes/app-routes.ts`, // relative import rewritten + `/@src/layouts/AppLayout.tsx`, // relative import rewritten } { if !strings.Contains(app, want) { - t.Errorf("app.ts missing %q", want) + t.Errorf("app.tsx missing %q", want) } } diff --git a/go/jsbundler/hmr_server.go b/go/jsbundler/hmr_server.go index 3502725f..6ca7abd8 100644 --- a/go/jsbundler/hmr_server.go +++ b/go/jsbundler/hmr_server.go @@ -65,6 +65,17 @@ func StartDevHMR(mux *http.ServeMux, cfg Config) (importMap string, err error) { return "", err } d.register(mux) + + // The watcher's seed scan only records starting mtimes and never fires + // onchange (see watch()), so anything already referenced in source before + // this boot — e.g. an icon name — would otherwise sit missing from the + // generated registry until some later edit happened to retrigger it. Run it + // once up front so a bare `go run -tags dev ./cmd/server` (skipping the + // "Bundle: Build" preLaunchTask) still starts consistent. + if err := generateFAIcons(); err != nil { + fmt.Fprintf(os.Stderr, "[hmr] generating FA icons: %v\n", err) + } + go d.watch() fmt.Println("HMR dev server: serving native-ESM source from /@src/, WebSocket at /@hmr/ws") diff --git a/go/jsbundler/hmr_watch.go b/go/jsbundler/hmr_watch.go index 63394d68..4678ee90 100644 --- a/go/jsbundler/hmr_watch.go +++ b/go/jsbundler/hmr_watch.go @@ -75,8 +75,10 @@ func (d *devServer) scan(roots []string, mtimes map[string]time.Time, onchange f func (d *devServer) handleChanges(changed []string) { pagesManifestAbs := filepath.Join(d.frontend, filepath.FromSlash(pagesManifest)) + faIconsOutAbs := filepath.Join(d.frontend, "src", "ui", "generated", "faIcons.ts") cssDirty := false srcDirty := false + iconsDirty := false for _, p := range changed { base := filepath.Base(p) @@ -102,9 +104,23 @@ func (d *devServer) handleChanges(changed []string) { d.hmrJS(p) cssDirty = true // a class may have been added/removed srcDirty = true + // The icon registry is itself a generated .ts file under srcRoot, so its + // own hot-reload (above) must not re-trigger a regeneration pass — that + // would bump its mtime and loop forever. + if p != faIconsOutAbs { + iconsDirty = true + } } } + if iconsDirty { + // A source edit may have referenced a new icon name; rescan so it's + // available without a manual `go run ./cmd/bundle`. The regenerated file's + // own mtime change is picked up and hot-reloaded on the next poll. + if err := generateFAIcons(); err != nil { + fmt.Fprintf(os.Stderr, "[hmr] regenerating FA icons: %v\n", err) + } + } if cssDirty { select { case d.cssTrigger <- struct{}{}: diff --git a/go/jsbundler/js.go b/go/jsbundler/js.go index 8acca65f..8773419e 100644 --- a/go/jsbundler/js.go +++ b/go/jsbundler/js.go @@ -26,15 +26,15 @@ func esbuildDefine() map[string]string { } // resolveEntryPoint returns the path (relative to frontendDir) of the -// SPA entry point, preferring app.ts over app.js. +// SPA entry point, preferring app.tsx over app.js. func resolveEntryPoint() string { - if _, err := os.Stat(filepath.Join(frontendDir, "src/app.ts")); err == nil { - return "src/app.ts" + if _, err := os.Stat(filepath.Join(frontendDir, "src/app.tsx")); err == nil { + return "src/app.tsx" } return "src/app.js" } -// bundleJS bundles the SPA entry (app.ts/app.js) into bundle.min.js. +// bundleJS bundles the SPA entry (app.tsx/app.js) into bundle.min.js. func bundleJS() (bundleStats, error) { return bundleJSEntry(filepath.Join(frontendDir, resolveEntryPoint()), "bundle.min.js") } @@ -145,7 +145,7 @@ func bundleJSEntry(entry, outName string) (bundleStats, error) { // drop leading `../` segments. esbuild writes paths relative to the output // file's directory; since the bundle lives in wwwroot/ and the sources live // in frontend/, every entry starts with `../frontend/`. Stripping the prefix -// yields project-rooted paths like `frontend/src/app.ts`. +// yields project-rooted paths like `frontend/src/app.tsx`. func stripSourcemapParentPrefix(mapPath string) error { data, err := os.ReadFile(mapPath) if err != nil { diff --git a/go/tw/tailwind.go b/go/tw/tailwind.go index 02d8d264..95b7c3f7 100644 --- a/go/tw/tailwind.go +++ b/go/tw/tailwind.go @@ -4435,7 +4435,7 @@ func (t *Theme) keysInNamespaces(themeKeys []string) []string { if !strings.HasPrefix(key, prefix) { continue } - if strings.Index(key[2:], "--") != -1 { + if strings.Contains(key[2:], "--") { continue } if isIgnoredThemeKey(key, namespace) { @@ -8989,7 +8989,7 @@ func createVariants(theme *Theme) *Variants { registerCompoundVariants(variants, theme) registerPseudoVariants(variants) - registerFunctionalVariants(variants, theme) + registerFunctionalVariants(variants) registerBreakpointVariants(variants, theme) registerMediaVariants(variants) @@ -9362,7 +9362,7 @@ func registerPseudoVariants(variants *Variants) { sv("inert", "&:is([inert], [inert] *)") } -func registerFunctionalVariants(variants *Variants, theme *Theme) { +func registerFunctionalVariants(variants *Variants) { variants.functional("aria", func(r *AstNode, variant *Variant) bool { if variant.Value == nil || variant.Modifier != nil { return false diff --git a/go/validation/validation.go b/go/validation/validation.go index 12bf3923..e9f8fbf8 100644 --- a/go/validation/validation.go +++ b/go/validation/validation.go @@ -198,6 +198,56 @@ func ValidateZipCode(zip string) error { return nil } +func ValidateEmail(email string) error { + err := errors.New("Invalid email address.") + + if email == "" { + return err + } + + emailParts := strings.Split(email, "@") + if len(emailParts) != 2 { + return err + } + + account := emailParts[0] + address := emailParts[1] + + lenErr := errors.New("Email address is too long.") + if len(account) > 64 { + return lenErr + } else if len(address) > 255 { + return lenErr + } + + domainParts := strings.Split(address, ".") + for _, part := range domainParts { + if len(part) > 63 { + return lenErr + } + } + + re := regexp.MustCompile(`^[-!#$%&'*+/0-9=?A-Z^_a-z` + "`" + `{|}~](\.?[-!#$%&'*+/0-9=?A-Z^_a-z` + "`" + `{|}~])*@[a-zA-Z0-9](-*\.?[a-zA-Z0-9])*\.[a-zA-Z](-?[a-zA-Z0-9])+$`) + if !re.MatchString(email) { + return err + } + + return nil +} + +func ValidateUsername(username string) error { + if len(username) < 5 || len(username) > 50 { + return errors.New("Username must have 5-50 characters.") + } + + isAlphanumeric, _ := regexp.MatchString(`^[A-Za-z0-9]*$`, username) + if !isAlphanumeric { + return errors.New("Username must only contain alphanumeric characters.") + } + + return nil +} + func ValidateUrl(url string) error { err := errors.New("Invalid URL.")