Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If breaks in advanced compilation #119

Closed
SevereOverfl0w opened this issue May 17, 2016 · 5 comments
Closed

If breaks in advanced compilation #119

SevereOverfl0w opened this issue May 17, 2016 · 5 comments

Comments

@SevereOverfl0w
Copy link

I believe I have encountered a bug in Sablono. Admittedly it occurs in coordination with Om, and the bug may be further upstream.

In advanced compilation mode, shared state is lost. The shared state comes in via a reference to the parent, which is set in a dynamic variable.

It only occurs when an if is around a for. Other things in place of the for may fail, but I'm not sure what to test with. If I have the om/build immediately in the if, it works okay.

I have created a test case here: https://github.com/SevereOverfl0w/sablono-conditional-bug

  • It uses Boot
  • To launch it, run boot dev
  • Watch for the clojurescript to finish compiling
  • Check out localhost:3000, the "stato" is nil and renders as a blank string ""
  • If you modify src/index.cljs.edn and change the optimizations to none, wait for cljs to build again, and refresh, the value from shared state becomes visible (dividita stato)
@r0man
Copy link
Owner

r0man commented May 17, 2016

@SevereOverfl0w Try adding a doall in front of the for. This is how I managed to get your example working. I suspect this has to do with lazy evaluation, and it could be a bug in sablono not realizing the lazy sequence.

I'm AFK until next week friday, so can't look into this right now.

If you want to dive into this issue, my suggestion is to look into places in the sablono sources where something like map or for is used to process the child elements of a node and producing a lazy seq. I think om does force lazy seqs at some places and sablono should do that too, to avoid these kind of bugs. :)

r0man

@SevereOverfl0w
Copy link
Author

Thanks for taking the time to reply to this issue, especially as you're taking some time AFK.

For now I've put a doall in the codebase where I first noticed this. Works really well, thanks!

@r0man
Copy link
Owner

r0man commented May 28, 2016

@SevereOverfl0w I added a fix for this issue. Seems to work on your test project. Could you please try out 0.7.2-SNAPSHOT from Clojars and confirm it is working for your project as well?

@SevereOverfl0w
Copy link
Author

Thanks for the fix, works excellently.

I have a minor concern with the snapshot, you may know about it already:
WARNING: Use of undeclared Var sablono.interpreter/element at line 94 /home/user/project/target/cljsbuild-compiler-0/sablono/interpreter.cljc

@r0man
Copy link
Owner

r0man commented May 31, 2016

@SevereOverfl0w Thanks for the confirmation. I fixed the warning, and released this as 0.7.2.

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

No branches or pull requests

2 participants