hoplon

The :hoplon: ClojureScript Web Framework - http://hoplon.io/
flyboarder 2018-01-02T00:07:22.000045Z

@thedavidmeister fixed!

2018-01-02T02:11:35.000041Z

@flyboarder hmm, that last point might cause me issues

2018-01-02T02:11:43.000056Z

i'll see if i can do what i need juggling some wrappers

2018-01-02T02:20:39.000089Z

yah Implementing IFn is a Hoplon feature and it's usage on native elements will upgrade them. causes my tests to fail

2018-01-02T02:25:23.000049Z

if using an element with hoplon overrides the protocol unconditionally then that makes hoplon unfriendly to 3rd party libs

2018-01-02T02:29:00.000086Z

actually nm

2018-01-02T02:29:10.000059Z

because you removed the protocol override at the same time it might not matter

2018-01-02T02:29:16.000047Z

hang on, i'll try some things out...

2018-01-02T02:40:28.000024Z

yes ok cool, the issues i used to have are gone in 7.2 so i can use hoplon elements with ckeditor5 now

flyboarder 2018-01-02T02:43:15.000084Z

@thedavidmeister woot!

2018-01-02T02:45:14.000096Z

it's pretty cool actually, i have a fn that takes a cell and an el and then builds an inline wysiwyg into the element that syncs with the cell

flyboarder 2018-01-02T02:45:59.000024Z

slick!

2018-01-02T02:46:00.000040Z

before it was a mix of native elements and hoplon stuff to avoid the protocol overrides, now it's all hoplon

flyboarder 2018-01-02T02:46:53.000104Z

awesome! the refactor was pretty straight forward, basically it wraps hoplon elements in ->hoplon to upgrade them

2018-01-02T02:48:26.000057Z

yeah i saw that

2018-01-02T02:49:22.000012Z

any idea what the overhead of calling specify! per-element vs. extending js/Element globally is?

flyboarder 2018-01-02T02:51:10.000150Z

the creation of the element might be slightly longer because specify! mutates the object, where the overrides did it once, however size of object should be the same

flyboarder 2018-01-02T02:51:53.000061Z

but i think it’s still better than using specify and cloning the object

2018-01-02T02:53:32.000034Z

well if we only support cell children management through public hoplon fns

2018-01-02T02:53:40.000115Z

can we just extend js/Element?

2018-01-02T02:53:46.000141Z

why do we need to do it per-element?

flyboarder 2018-01-02T02:56:02.000056Z

if we dont do it per hoplon element the elements arnt upgraded and ready for template macros

2018-01-02T02:57:41.000012Z

ok i think i need to read what you did more closely

flyboarder 2018-01-02T02:57:47.000016Z

there is no way for a native element to be “aware” of cell children unless it’s upgraded, doing so will give DOM Exception 8 (not an element)

flyboarder 2018-01-02T02:58:47.000101Z

so to have hoplon elements be “ready” for cells they need to be upgraded a head of time

flyboarder 2018-01-02T02:59:37.000001Z

to make tests pass for native parents, we pass them through the public API instead of using the protocol methods

flyboarder 2018-01-02T03:07:09.000037Z

thats probably why we extended the prototypes in the first place, to make all elements support cell children, but that implicit behaviour breaks 3rd party libs

2018-01-02T03:13:20.000046Z

yes, it's also probably much slower than native fns

2018-01-02T03:13:56.000050Z

i guess i'm asking why does anything need to be "aware" or "ready" of anything?

2018-01-02T03:14:18.000025Z

just use .appendChild() if you want native behaviour and append-child! if you want hoplon managed behaviour

flyboarder 2018-01-02T03:14:44.000029Z

thats correct

2018-01-02T03:14:54.000106Z

so why do we need to "upgrade" anything?

flyboarder 2018-01-02T03:15:34.000060Z

to support cells as childred in the form (div :someattr someval (*-tpl))

flyboarder 2018-01-02T03:15:58.000022Z

otherwise you will get not element exception

2018-01-02T03:16:08.000100Z

why not implement IHoplonElement for js/Element then use append-child! to implement everything on top of that?

flyboarder 2018-01-02T03:18:14.000102Z

well by implementing on js/Element, we are extending every element, not just hoplon elements, the intention was to avoid doing anything to native elements, but if it’s faster to modify them all we can do that too

2018-01-02T03:18:40.000030Z

extending is fine IIRC

2018-01-02T03:18:46.000004Z

it was overriding that caused issues

2018-01-02T03:19:43.000108Z

no third party lib is going to be expecting append-child! to exist or work a specific way, unless it is hoplon aware anyway

flyboarder 2018-01-02T03:19:46.000038Z

mhm we still extend, but on element creation instead of globally

2018-01-02T03:20:21.000141Z

but obviously everything wants to use the protocol and expects it to work exactly as the spec

2018-01-02T03:21:00.000074Z

what i'm saying, is that why even have the idea of "a hoplon element"?

2018-01-02T03:21:27.000110Z

why not just extend all elements once, globally

flyboarder 2018-01-02T03:22:42.000007Z

we could do that, however the problem is still the same, how do you implement cells as children, either a hoplon element that can handle the children or a “JavelinCellElement” which would be an element implementing the cell protocol

flyboarder 2018-01-02T03:23:17.000084Z

you have to be “aware” on at least one side of the equation

flyboarder 2018-01-02T03:24:22.000038Z

we can extend globally but then we need to track separately which elements implement the hoplon features, back to property checking

2018-01-02T03:24:26.000077Z

cells as children doesn't even totally work right now

flyboarder 2018-01-02T03:25:50.000035Z

currently we are expecting the cell to be dereferenced before passed to those, as per the code, otherwise we need to check for an deref the cells as they come in

2018-01-02T03:27:58.000023Z

right, so the element isn't really handling cells as children

2018-01-02T03:28:43.000012Z

it's more like, there's an adapter that watches cells and manipulates the element with the cell's value (which is dom elements)

flyboarder 2018-01-02T03:29:08.000038Z

right but that is tied to the element as a method

2018-01-02T03:31:07.000047Z

does it have to be? could things be juggled so that el and children are args to a fn with a scope, rather than having a property on the el itself?

2018-01-02T03:34:27.000037Z

(defn with-children
 [el children]
 (j/with-let [el el]
  (do-watch children
   (fn [o n]
    ... diff o and n and update el ...))))

2018-01-02T03:34:36.000066Z

sort of like this

flyboarder 2018-01-02T03:34:37.000002Z

I see what you are saying, that could probably work, we could move that atom to a let instead around the element, however that could cause garbage collection issues when the element is destroyed

2018-01-02T03:40:39.000020Z

mmm is it possible to implement IWatchable for dom elements?

flyboarder 2018-01-02T03:41:01.000001Z

I dont see why not

2018-01-02T03:41:47.000070Z

could we do some cleanup that would help gc if we saw the element be destroyed?

flyboarder 2018-01-02T03:43:38.000029Z

not sure, I would say this relates to https://github.com/hoplon/hoplon/issues/127

2018-01-02T03:57:18.000060Z

reading...

2018-01-02T03:58:14.000023Z

on a sidenote though, even if we keep the data on the element it might make sense to implement it as metadata

2018-01-02T04:00:05.000130Z

that seems to be how cljs native does stuff like specify!, memoize, etc.

2018-01-02T04:03:36.000040Z

yeah this nesting question is unsolved currently though

2018-01-02T04:08:02.000126Z

well either way, i'm not seeing any perf issues just clicking around a bit

2018-01-02T04:13:11.000052Z

i'm probably just prematurely optimising 😛

flyboarder 2018-01-02T04:24:17.000057Z

7.1: {:id :app, :n-calls 5, :min "6ms", :max "50ms", :mad "13.76ms", :mean "15.6ms", :time% 100, :time "78ms"} 7.2: {:id :app, :n-calls 5, :min "9ms", :max "40ms", :mad "9.12ms", :mean "17.2ms", :time% 100, :time "86ms"}

flyboarder 2018-01-02T04:24:21.000006Z

@thedavidmeister ^

flyboarder 2018-01-02T04:24:29.000088Z

render an app 5 times

2018-01-02T04:25:42.000096Z

it is about 10% slower

2018-01-02T04:26:05.000004Z

not disastrous but not ideal either

flyboarder 2018-01-02T04:30:36.000063Z

I think thats acceptable for a new internal implementation, we can improve for performance moving forward

flyboarder 2018-01-02T04:30:54.000058Z

considering it doesn’t break anything or require changes

2018-01-02T04:30:54.000060Z

i think the pros outweigh the cons yes

2018-01-02T04:31:19.000068Z

overriding the protocol is a real pandora's box for 3rd party compatibility

2018-01-02T04:34:16.000025Z

ah also, i left some comments on a review

2018-01-02T04:34:23.000003Z

java.lang.ClassCastException: java.lang.Boolean cannot be cast to clojure.lang.IFn

2018-01-02T04:34:29.000049Z

i'm seeing this on staging builds

2018-01-02T04:35:48.000100Z

you've accidentally shadowed bust-cache

2018-01-02T04:36:36.000004Z

so tests pass but builds fail

2018-01-02T04:43:33.000098Z

@flyboarder if you fix ^^ can you push a new snapshot to clojars so i can re-build?

flyboarder 2018-01-02T05:13:59.000086Z

@thedavidmeister pushing…

flyboarder 2018-01-02T05:23:58.000083Z

I found that removing cells doesnt work because we lose the reference key for the watcher, if we store this in meta data on the cell we could pull it out again

2018-01-02T06:08:30.000020Z

i have done similar things using meta on cells, e.g.

2018-01-02T06:08:33.000134Z

(defn debounce-cell?
 [c]
 (and
  (j/cell? c)
  (::debounce-cell-ms (meta c))))

(defn debounce-cell
 "debounce a non-debounce cell"
 [c ms]
 {:pre [(j/cell? c)
        ; debouncing an already-debounced cell is almost certainly a mistake
        (not (debounce-cell? c))
        (number? ms)]
  :post [(debounce-cell? %)]}
 (j/with-let [r (if env.data/testing?
                 ; don't want to deal with debounce making almost everything async on CI.
                 (j/cell= c)
                 (let [t (j/cell nil)]
                  (j/with-let [d (j/cell @c)]
                   (add-watch c (gensym)
                    (fn [_ _ _ n]
                     (.clearTimeout js/window @t)
                     (reset! t
                      (h/with-timeout ms
                       (reset! d n))))))))]
  (alter-meta! r assoc ::debounce-cell-ms ms)))

(defn safe-debounce-cell
 "debounce a cell, or return an already debounced cell if ms is compatible"
 [c ms]
 {:post [(debounce-cell? %)]}
 (cond
  (not (debounce-cell? c))
  (debounce-cell c ms)

  (= ms (::debounce-cell-ms (meta c)))
  c

  :else
  (throw (js/Error. "Attempted to debounce a debounce cell with incompatible ms."))))

2018-01-02T06:09:44.000070Z

this might be a 7.3 thing though?

2018-01-02T06:09:59.000098Z

7.2 is already getting biggish

flyboarder 2018-01-02T06:12:50.000041Z

yeah, I think 7.2 is largely complete, need to do house keeping on the related tickets

flyboarder 2018-01-02T06:13:20.000122Z

https://github.com/hoplon/hoplon/milestone/3

2018-01-02T07:04:53.000179Z

clojure.lang.ArityException: Wrong number of args (2) passed to: impl/prerender/->frms--6522

2018-01-02T07:04:58.000045Z

^^ from new build

2018-01-02T07:10:02.000070Z

(->frms in-path slurp) should be (->frms (slurp in-path)) i think

flyboarder 2018-01-02T07:16:33.000125Z

where?

2018-01-02T07:17:45.000081Z

i'll put a comment up

2018-01-02T07:25:09.000015Z

lol, <h1 "Embedded Hoplon example"/> whoever wrote this is using hoplon too much

2018-01-02T07:46:42.000149Z

#178 is fixed by 7.2.0-SNAPSHOT! 🎉

2018-01-02T07:48:31.000036Z

@mudphone can you check 7.1.0 stable release too?

2018-01-02T07:49:41.000018Z

doing that now … 🙂

2018-01-02T07:50:25.000055Z

It works on my machine ™️ . 🎉

2018-01-02T07:52:15.000111Z

great

2018-01-02T07:52:24.000177Z

must have been some issue in one of the snapshots

2018-01-02T07:53:02.000047Z

actually we did fix some of the prototype overrides in 7.1, could have easily been that

2018-01-02T07:53:31.000186Z

then @flyboarder took it to the next level in 7.2

2018-01-02T08:04:50.000137Z

great, thanks!

2018-01-02T08:12:15.000179Z

I’m guessing the “prototype overrides” fix might be a bit over my head, without a lot of explaining?

2018-01-02T08:14:42.000104Z

not a lot of explaining really

2018-01-02T08:15:02.000067Z

hoplon was just overriding some low level global functions that other libs expect to work in a certain way

2018-01-02T08:15:14.000052Z

which obviously works... until it doesn't 😉

2018-01-02T08:15:51.000036Z

in 7.1 we changed it to only override those fns if the element was created by hoplon

2018-01-02T08:15:57.000053Z

in 7.2 there are no more overrides

2018-01-02T08:16:06.000186Z

ah cool

2018-01-02T08:16:30.000008Z

I owe you guys one. You can collect at the next Hoplon conference in Hawaii.

2018-01-02T08:16:36.000057Z

haha yup

2018-01-02T08:16:48.000253Z

well it was breaking my wysiwyg editor too 😛

2018-01-02T08:18:13.000007Z

Ha ha, cool. Well, it sounds real clean now.

2018-01-02T08:18:57.000165Z

working on it...

2018-01-02T08:21:54.000149Z

had to give up a little "magic" but in theory hoplon plays much nicer with libs now 🙂

1👍
2018-01-02T08:41:17.000281Z

@flyboarder do you know what this is about?

2018-01-02T08:41:21.000071Z

(when-not (:static attrs)
    (merge-kids elem nil nil)
    (add-children! elem kids)))))

2018-01-02T08:41:35.000118Z

or @micha @alandipert ^^ in mksingleton

2018-01-02T09:12:42.000275Z

why do we need mksingleton at all?

2018-01-02T09:13:10.000113Z

(defn head
 [& args]
 (apply (.-head js/document) args))

2018-01-02T09:13:42.000128Z

wouldn't this work using standard IFn logic? ^^

flyboarder 2018-01-02T20:50:14.000084Z

I moved a few issues to 7.3 milestone as they would require more refactoring.