Skip to content

Simplify & clean up viewers#53

Merged
mk merged 10 commits into
mainfrom
simplify-viewers
Jan 13, 2022
Merged

Simplify & clean up viewers#53
mk merged 10 commits into
mainfrom
simplify-viewers

Conversation

@mk

@mk mk commented Jan 13, 2022

Copy link
Copy Markdown
Member

Simplify the viewer api by dropping the macros and requiring users to quote the :render-fn. This should make it explicit and less magic as to what part of the viewers is evaluated on the JVM and what is sent over the wire to run in the browser.

@mk

mk commented Jan 13, 2022

Copy link
Copy Markdown
Member Author

@zampino @philomates if either of you want to look over this before I merge it, please do.

@philomates philomates left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

Comment on lines +12 to +14
#?@(:cljs [IFn
(-invoke [this x] ((:f this) x))
(-invoke [this x y] ((:f this) x y))]))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so on the :clj side we no longer want to implement IFn, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right.

Comment thread src/nextjournal/clerk/viewer.cljc Outdated
(def elide-string-length 100)

(declare with-viewer*)
(declare with-viewer)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this line can be removed completely

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, dropped it in ac9dee5.

{:pred string? :render-fn (quote v/string-viewer) :fetch-opts {:n 100}}
{:pred number? :render-fn '(fn [x] (v/html [:span.tabular-nums (if (js/Number.isNaN x) "NaN" (str x))]))}])

(defn process-fns [viewers]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@mk mk merged commit e4acb37 into main Jan 13, 2022
@mk mk deleted the simplify-viewers branch January 13, 2022 20:13
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