bhauman / devcards

Devcards aims to provide a visual REPL experience for ClojureScript
1.53k stars 113 forks source link

Simplify defcard-om-next #103

Closed bharrisau closed 6 years ago

bharrisau commented 8 years ago

I'm a little confused with the defcard-om-next macro. When used with a reconciler, the initial-data parameter is never used. (https://github.com/bhauman/devcards/blob/master/src/devcards/core.clj#L189) When the reconciler variable is data, the data is used for the reconciler initial state but isn't actually used by the card because initial-data is always returned from the parser.

It also looks like parse-card-args only returns 5 args, not 6.

I'd suggest updating the macro to combine the om-next-reconciler and initial-data and switch on them using reconciler?

Something like this (excuse my poor macro knowledge)


(defmacro defcard-om-next [& exprs]
  (when (utils/devcards-active?)
    (let [[vname docu om-next-comp initial-data options] (parse-card-args exprs 'om-next-root-card)
           om-next-reconciler (if (reconciler? initial-data) initial-data nil)
           om-next-data (if (reconciler? initial-data) nil initial-data)]
      (card vname docu `(om-next-root ~om-next-comp ~om-next-reconciler) om-next-data options))))
bhauman commented 8 years ago

Yeah this deserves a better look.

I haven't invested the needed time into OmNext to make informed decisions here. AFAIK the goto implementation for om-next devcards is the project linked below and I'm just waiting for things to settle down before integrating it. Maybe this should happen soon.

My suggestion is to try this project and make commits and improvements to it and then we will integrate it straight into devcards.

https://github.com/anmonteiro/devcards-om-next