nextjournal / clerk

⚡️ Moldable Live Programming for Clojure
https://clerk.vision
ISC License
1.74k stars 75 forks source link

Analyzer Improvements #629

Closed zampino closed 4 months ago

zampino commented 5 months ago

Fixes #454 Fixes #628 Fixes #66 Fixes #634

zampino commented 5 months ago

Some edge cases still need to be addressed:


(def a 1)

(def a (inc a))

a

the analyzer/analyze fails to return a as dependency of the (def a (inc a)). This is because we set-subtract all introduced vars from the deps: https://github.com/nextjournal/clerk/blob/38a4653492bd47799f6dc5e6864e2b7eaf47a410/src/nextjournal/clerk/analyzer.clj#L187 Not doing that leads to other issues (e.g for multi-arity defns (defn f ([x] (f {} x)) ([x opts] ...)).

So when the first definition changes, this won't propagate to the value of a at the bottom. It's still an improvement wrt main, where the code above just crashes (#634).

zampino commented 5 months ago

Second case:

(def a 1)

(inc a)

(def a :boom)

Similarly, this also crashes on main (#634) but evals correctly a first time on this branch.

Subsequent clerk/show! fail when the consumer of the var a changes, since a is bound to a keyword, but (def a 1) didn't change. A similar behaviour to the "deref deps" might be applied here, but we need to decide when.

zampino commented 4 months ago

Of course I didn't thought of the example:

(def a 1)
(when false (def a 2))
(inc a)

so also the current handling of redefinitions in the graph won't work reliably, I'm tempted to not cache definitions with the same name at all and let people decide (throw on redefs, warn+not-cache, not-cache).

zampino commented 4 months ago

In this other branch I've disabled caching for redefined vars (and therefore for their dependants). We might mark in the UI the blocks that are not cached and in case of the redefinitions explain why.

zampino commented 4 months ago

Trying out passing info to code cells

https://github.com/nextjournal/clerk/assets/1078464/7f11c5db-3243-4681-bed5-1729a94e759a