lilactown / helix

A simple, easy to use library for React development in ClojureScript.
Eclipse Public License 2.0
631 stars 52 forks source link

using clojure.spec with components #57

Closed lilactown closed 1 year ago

lilactown commented 4 years ago

clojure.spec would be very useful as a way to document props and assert at runtime that they are correct.

There are two fundamental issues that I would want to address by providing direct support for clojure.spec:

A rough sketches of a potential API:

(ns app.feature
  (:require
    [clojure.spec.alpha :as s]
    [helix.core :refer (defnc $)]
    [helix.dom :as d]
    [helix.spec :as hs]))

;; can use regular specs
(s/def ::name string?)

;; `hs/def` is just like `s/def` but will be removed if goog/DEBUG is false
(hs/def ::on-name-change function?)

;; like `s/fdef` but for components
(hs/cdef greeting
  :props (s/keys :req-un [::name ::on-name-change]))

;; specs are separate from components, can be added on later or removed w/o
;; any change to component.
;; Later maybe add specific syntax for speccing inline?
(defnc greeting
  [{:keys [name on-name-change]}]
  (d/div
    (d/div “Hello, “ name “!”)
    (d/input {:value name :on-change on-name-change})))
lilactown commented 4 years ago

@SevereOverfl0w brought up that it would be easy enough to have helix.spec simply expose a props spec combinator that could take a predicate / spec / spec-identifier, a la:

(s/fdef greeting
  :args (s/cat :props (hs/props map?)))

That seems pretty minimal. Additional helpers could be added on to that if valuable.

wilkerlucio commented 4 years ago

If you go for internal syntax, I like to suggest using the same from Guardrails/Ghostwheel.

khmelevskii commented 4 years ago

@lilactown yes, it's a great idea! I did something like this. I created defnc macro based on helix/defnc. I'm able to pass clojure.spec to this macro for validation component props and render warning in browser console or generate exception during the tests if props is not valid.

For example:

(s/def ::class-name string?)
(s/def ::size #{300 200})
(s/def ::children ::s.common/children)

(s/def ::props
  (s/keys
   :req-un [::children]
   :opt-un [::class-name ::size]))

(defnc <Badge>
  [{:keys [class-name size children]}]
  {:spec ::spec/props}
  "badge")

In browser console I can see the following messages https://nimb.ws/4Pw3KR

Also in production build I remove spec checks for components.

tekacs commented 4 years ago

To make a (hopefully small) request -- I'd love to ask that you make spec validation pluggable (or at least overrideable) if you add an internal implementation?

I use malli instead of spec and would love to be able to swap out the spec validator for malli's explainer instead!

dvingo commented 1 year ago

I've been using the delegate pattern where defnc calls out to a function: https://github.com/matterandvoid-space/todomvc-fulcro-subscriptions/blob/766d27be316c3f2ab6a23bd8db30932ec0601a4f/src/main/space/matterandvoid/todomvc/todo/ui.cljs#L13 so that I can use malli instrumentation similar to react's propTypes

@khmelevskii I'm curious - does your macro emit another function for the body so that the specs receive cljs types instead a js object?

riotrah commented 1 year ago

I thought y'all might be interested in this in the interim.

Personally, also awaiting a convenient way to swap out arbitrary "type checking systems" at will between spec, malli/schema or React.PropTypes or whatever. The delegation pattern is quite interesting I have to say!

lilactown commented 1 year ago

I think that discussion of this can continue in helix-spec-alpha that @riotrah posted above. Closing this for now!

dvingo commented 1 year ago

For anyone using malli I found a really nice way to integrate helix with malli to get function instrumentation of helix components: https://github.com/metosin/malli/pull/899

I'm using this currently in my fork of malli. If that PR isn't merged for whatever reason you can also just copy the -map-schema implementation from malli.core and add that :xf option to your own registry.

dvingo commented 1 year ago

The above idea for getting malli instrumentation support didn't work out. Instead I found a solution by changing the defnc implementation to emit two functions - the inner function will be instrumented correctly by a normal :map schema:

https://gist.github.com/dvingo/97bae8b33c08b257153946bb82f38a86

rome-user commented 1 year ago

@dvingo Very nice work... is there somewhere we can discuss this further? I do not quite understand the need to emit an function after the first def. Would be nice to briefly explain in a comment.

dvingo commented 1 year ago

@dvingo Very nice work... is there somewhere we can discuss this further? I do not quite understand the need to emit an function after the first def. Would be nice to briefly explain in a comment.

I suppose you can comment on the gist? The goal is to support using [:map [:key1 :string] [:key2 :int]] etc malli syntax to describe helix props and get malli instrumentation to work with that. The issue is that if you attempt to instrument a plain helix defnc directly (which emits a function, so can be instrumented) the arguments to that function are a JS object, which will not work with the instrumentation - malli expects a hashmap or something that implements its protocols (cljs bean). The custom macro I wrote will instrument a nested function so that as a user you can go on pretending you're getting a hashmap to your helix component and malli can correctly validate the arguments. Does that make sense? - feel free to leave comments on the gist.