This ☝️ PR adds a provider that uses the dom apis directly. I added an experimental :smart-class attribute that can remove classes when using strings or vectors. But it doesn't compose well as multiple calls would remove classes that shouldn't be removed.
Wouldn't it make sense to load that dom namespace by default? It's not like it's going to add much extra weight to the library? I wonder if removing the multimethod indirection altogether would make stuff perform better. It seems the multimethod was only there to offer a choice between jquery and goog but now that isn't necessary anymore?
I'm not sure if applied-science.js-interop adds more size to the bundle, but probably not since they are mostly macros that emit direct JS interop code when used properly
I did a build report and the dom provider is smaller than goog and js-interop adds 45 bytes.
this 45 bytes is probably a mis-representation, because it's just a dummy module that refers to its real function in another module
you have to expand all the sections to see where those functions were moved
I can take a look if you tell me how to build
> Wouldn't it make sense to load that dom namespace by default? It's not like it's going to add much extra weight to the library? I wonder if removing the multimethod indirection altogether would make stuff perform better. It seems the multimethod was only there to offer a choice between jquery and goog but now that isn't necessary anymore? That would be a potential breaking change for projects that depend on jquery that is the current default so I want to make sure that this is working as it should before changing it. Multimethods predate the goog provider and are a mechanism for users to add custom attributes and I'm not sure of a good way to provide that without using them (considering the ergonomics of using this extension mechanism).
understood
> I can take a look if you tell me how to build
The way I'm testing this is not something I'm proud yet but it works 😅
• clone hoplon locally https://github.com/hoplon/hoplon and checkout the dom-provider branch
• clone hoplon-demos locally https://github.com/hoplon/demos and checkout the hoplon-local-counter branch.
• go to the counters directory on hoplon-demos:
◦ npm install
◦ npx shadow-cljs watch app
I mean the build report, you do this with npx shadow-cljs build app with some flag?
I didn't port that since this file wasn't reachable from nextjournal.clojure-mode
but I can have a stab at it
whoops wrong channel
The file src/demo/counter.cljs on the counters demo has the requires for the providers. I comment the providers that I don't want and leave only the one I want.
To build the report the command is npx shadow-cljs run shadow.cljs.build-report app report.html
you're right, only 45 bytes for applied-science.js-interop, amazing :)
well, good job, hope the PR gets merged
I think that the core namespace could be bigger because of the macro expansion, but not sure about how to confirm this.
I think the js-interop macros do their best to generate compact code
although in some cases, like get-in you will get bigger code because of nil checks:
cljs.user=> (macroexpand '(j/get-in x [:foo :bar]))
(let* [obj1617 (clojure.core/let [obj1616 x] (if (clojure.core/some? obj1616) (cljs.core/unchecked-get obj1616 "foo") js/undefined))] (if (clojure.core/some? obj1617) (cljs.core/unchecked-get obj1617 "bar") js/undefined))> well, good job, hope the PR gets merged Thanks! It will. I will revert the namespace change on the goog provider and will merge it soon and make a release.
rather than (.. x -foo -bar)
I could use !get-in I think.
cljs.user=> (str (fn [x] (.. x -foo -bar)))
"function (x){\nreturn x.foo.bar;\n}"that is by far the shortest you will get
also slightly more performant
I probably should remove js-interop too. I will do before merging.
it is convenient, but maybe for a library it's nice to have little dependencies
I wanted to replace goog.obj/set. I will see what would be other options for that. To access attributes, I will not use js-interop anymore.
you can do this by using unchecked-set
(unchecked-set obj "foo" 1)cljs.user=> (str (fn [obj] (unchecked-set obj "foo" 1)))
"function (obj){\nreturn (obj[\"foo\"] = (1));\n}"or:
cljs.user=> (str (fn [obj] (set! (.-foo obj) 1)))
"function (obj){\nreturn (obj.foo = (1));\n}"Oh, this is great, thank you!
Is there a difference between set! and unchecked-set?
unchecked-set bypasses advanced compilation, but I think if you add a ^js type hint for the argument you pass to set it will also do the same. shadow-cljs will warn you if it can't infer that it is a JS value or not
testing with advanced compilation in CI is a necessity imo
Yeah, I added tests using advanced compilation to the ci in this pr for all providers.
👍