Skip to content

feature request: gate ->css on "style" key - #16

Merged
borkdude merged 2 commits into
borkdude:mainfrom
telekid:patch-1
Jun 13, 2026
Merged

borkdude merged 2 commits into
borkdude:mainfrom
telekid:patch-1

Conversation

@telekid

@telekid telekid commented May 27, 2026

Copy link
Copy Markdown
Contributor

In my application, I use records as values in form submissions. Currently, when I use those records with borkdude.html, I have to wrap those records in str:

(require '[borkdude.html :refer [html]])

(defrecord QuestionId [value]
  Object (toString [_] value))

(def qid (->QuestionId "f33b44d7-1b1c-4dab-8bb2-97622640f1a2"))

;; A defrecord satisfies `map?`, so it hits the (map? v) -> ->css branch
(html [:input {:type "hidden" :name "question-id" :value qid}])
;; =>  <input type="hidden" name="question-id" value="value: f33b44d7-1b1c-4dab-8bb2-97622640f1a2;">

(html [:input {:type "hidden" :name "question-id" :value (str qid)}])
;; =>  <input type="hidden" name="question-id" value="f33b44d7-1b1c-4dab-8bb2-97622640f1a2">

I find myself forgetting to add those wrappers on occasion. I just checked Hiccup, it looks like its semantics match the current behavior, so maybe this isn't the right move: https://github.com/weavejester/hiccup/blob/master/src/hiccup/compiler.clj#L57-L76

Another point of contention is whether we would want to wrap v in escape-html first.

LMK if this is of interest to you. If so, I can add a couple of tests.

@borkdude

Copy link
Copy Markdown
Owner

I think that would be reasonable

@telekid

telekid commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

This fix is ready

@borkdude

Copy link
Copy Markdown
Owner

Considering that React/Reagent only treat "style" as a CSS map too, I think this is good change.
On your escaping question: yes, please wrap in escape-html since you can still break things this way.

So:

:else (pr-str (escape-html (str v)))

While you're at it, the (pr-str (->css v)) branch has the same pre-existing issue ({:style {:color "blue\""}}), wrapping that one too would be nice.

A test with a quote in the record's toString would lock down the escaping behavior.

@telekid

telekid commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

See new changes

@borkdude

Copy link
Copy Markdown
Owner

Test failures though?

@telekid

telekid commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Oh, whoa, I did not expect a clj vs cljs difference here! Taking a look.

@telekid

telekid commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Found it. Funny cljs-only issue with escape-html, I think:

[jake@fedora]~/development/html% bb test:cljs

Testing borkdude.html-test

FAIL in (test-escaped-chars) (/home/jake/development/html/test/borkdude/html_test.cljc:127:9)
expected: (= (escape-html "\"\"") "&quot;&quot;")
  actual: (not (= "&quot;\"" "&quot;&quot;"))

Investigating.

@telekid
telekid force-pushed the patch-1 branch 2 times, most recently from 321a9d3 to 8d2222d Compare June 12, 2026 16:11
@telekid

telekid commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Okay, tests appear to pass now.

@borkdude
borkdude merged commit 9420f58 into borkdude:main Jun 13, 2026
1 check passed
@borkdude

Copy link
Copy Markdown
Owner

Thanks!

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.

2 participants