hashobject / perun

Programmable static site generator built with Clojure and Boot (HELP NEEDED!)
https://perun.io
Eclipse Public License 1.0
350 stars 38 forks source link

require renderer ns before evaluating with arguments #171

Closed martinklepsch closed 7 years ago

martinklepsch commented 7 years ago

I have a renderer in app.static that eventually gets some records as arguments. The records are defined in namespace app.records. app.static transitively requires app.records.

When the renderer is called with it's arguments (the records) the arguments seem to be deserialized before the renderer-fn is called. Because of this I get an exception like this one:

java.lang.ClassNotFoundException: app.records.Track

I solved the issue manually now by running the following before the task is executed:

(boot.pod/require-in @perun/render-pod 'app.static) ; transitively requires app.records

but not having to do that would be nice. For some reason the require-in docs state "Avoid this function." but I'm not sure why. Maybe @micha or @alandipert can weigh in on that.

bhagany commented 7 years ago

Huh, that's interesting. The render function is called with with-call-in, which does require the renderer namespace here: https://github.com/boot-clj/boot/blob/master/boot/pod/src/boot/pod.clj#L357. (The call chain is a little hard to follow, so it goes: with-call-in -> call-in* 2-arity -> call-in* 1-arity -> eval-fn-call). So the error must be happening before it gets there.

I was under the impression that Boot didn't support things that don't round-trip through the reader, so I'm kind of surprised this works at all...

martinklepsch commented 7 years ago

I was under the impression that Boot didn't support things that don't round-trip through the reader, so I'm kind of surprised this works at all...

Records roundtrip the reader similarly to how maps do afaik.

Equally confused about the call stack. I think the issue is that in eval-fn-call the args are already evaluated/deserialized and just at that moment the fn namespace is required.

bhagany commented 7 years ago

Ah, found it: "Only forms that can be printed with pr-str and read via read-string can pass between pods." from https://github.com/boot-clj/boot/wiki/Boot-Troubleshooting.

Records are kind of an in-between thing that can sometimes meet that criteria, depending on what things are required... I'm going to ask for opinions on this on Slack.

martinklepsch commented 7 years ago

@bhagany I think this issue might really belong to boot-clj/boot but we should check with @micha...

bhagany commented 7 years ago

Yeah, I think it could go either way. It's an interesting issue :)

bhagany commented 7 years ago

okay, I talked this over with @micha on Slack, and I have a few avenues to explore for a solution. The upshot is, we'll be able to support this in Perun.

bhagany commented 7 years ago

Sorry this took so long. I spent a while trying to understand boot.core/with-pod, hoping that I could make it fit this use-case, but that didn't work out. This also doesn't solve the problem with records in print-meta. I'm unsure how best to do that at the moment

bhagany commented 7 years ago

Idea: modify print-meta to take a vector of namespaces to require in the pod. What do you think?

martinklepsch commented 7 years ago

I guess that would work but it reveals an implementation detail and requires explicit dealing with it. That said I don't really have a better idea so maybe it's the best possible? (Also I guess this custom record thing is probably not very common so it may only affect a small number of people.)

bhagany commented 7 years ago

I suppose it's not really much better than using require-in like you're doing. I'll keep thinking about it.