Refactor nbJs

[HugoGranstrom]

As has been discussed in #118, the gensym-logic of nbJs is too complicated for the basic use-cases. Karax needs this advanced logic though, as each karax component must be compiled in its own file.

So the goal for this PR is to make the normal nbJsFromCode only use blocks instead gensym-logic. I have already started and I am getting nice behavior for simple cases. I need to check many more examples, though.

This PR will introduce several breaking changes:

• nbJsFromCode will put the code inside a block so imports will not be allowed in them. Instead, you must use nbJsFromCodeGlobal which will be put at the top of the final script.
• nbJsFromCodeInit, addCodeToJs will be deprecated as we are limiting ourselves to isolated blocks. If you want interop between multiple nbJs blocks, define a global variable using nbJsFromCodeGlobal.

Now I'm going to try and fix the docs.

TODO:
from in-code review:

• first 4 remarks are action to be taken, small details, when we close them we resolve the conversation
• decision to take: do we interleave global scope and blocks or not?
  • Yes! We have 3 different kind of blocks including a interleaved global block and one placed at the top.
• change logic of bump gensym, likely adding a counter and looping inside the macro. 👍

rest of conversation.

• decision to take: should we do something about possibility of showing code in JsFromCode?
  • Yes we will place each code block in its own NbBlock and use a collector block at the end. The old logic should still work for showing the code.
• move compileNimToJs from renders.nim to jsutils.nim
• Update Docs
  • first example will not be with capture variables
  • will add a internal working sections on sources of complexity
  • example on how to show appreciation about single js script with global and block sections.

[HugoGranstrom]

I have tested this new slim nbJs on our documentation and our NimConf slides now and it seems to work. Tomorrow I'll go through your nblog posts which uses nbJs. Specifically there was one which needed exportc to work. Will be interesting to see how it works out

[HugoGranstrom]

I checked nblog now and everything seems to work if we just move the imports to a global block 🥳

[HugoGranstrom]

Documentation and tests have been updated now. It's actually quite impressive how short jsutils.nim is now that we have thrown away all the gensym logic 🤯

I actually think this PR might be ready for review :o @pietroppeter

[pietroppeter]

Have you tried running the three_js example in nblog?

[HugoGranstrom]

Yes, moving the imports to a global block gives me a rotating purple cube :D
Bouncing_ball with the exportc works as well.
The plant app work.

[pietroppeter]

I am thinking that we might want to have also a block for when you plan to have a single (nim derived) script in the entire page.
We could have a template that does not require a separate global block. Should be easy to add, right? name? nbJsFromCodeSingle? nbJsFromCodeOnlyOne?

[HugoGranstrom]

I don't quite understand what you mean. We have nbJsFromCodeOwnFile which karaxCodeBlock uses internally which compiles a code block the old way into its own file. Is that what you mean?

[pietroppeter]

Without Gensym so that a case like threes_js works without a separate global section

[HugoGranstrom]

Nothing uses gensym anymore. So for this case you could either use nbJsFromCodeOwnFile or nbJsFromCodeGlobal and just put all your code in there directly. Both ways you won't put the code inside a block.

[pietroppeter]

ah ok, not even karax block use gensym? so two identical karax blocks will not run into issues of sharing variables?
anyway yeah I did not realize that I could just use nbJsFromCodeGlobal for everything.

and I need still to actually review code and understand for real this time how all this js stuff works. 🙄
in the back of my mind I still have the old question: why can't I just write code in a separate nim file, compile with js backend and put the output in a script tag in the document? I know it is wrong for many reason, but I still not really understanding where the different pieces of current complexity come from (I know about colliding identifiers when using more than one script, issue that become even worst when the script is basically the same - e.g. a widget, karax based or not, I know of captured variables requiring effort, not sure if there is other stuff).

thanks for all the gentle explanations! ❤️

[HugoGranstrom]

ah ok, not even karax block use gensym? so two identical karax blocks will not run into issues of sharing variables?

No, the bumpGensym I talked about yesterday in #118 is implemented and solved the problem :D It is gone forever! Look at the diff for jsutils.nim and enjoy all the red B)

and I need still to actually review code and understand for real this time how all this js stuff works. roll_eyes

The code has never been as comprehensible as it is now, so now is a good time to dig in. Keep shooting questions at me, I'm not merging this until you understand it ;)

in the back of my mind I still have the old question: why can't I just write code in a separate nim file, compile with js backend and put the output in a script tag in the document? I know it is wrong for many reason, but I still not really understanding where the different pieces of current complexity come from (I know about colliding identifiers when using more than one script, issue that become even worst when the script is basically the same - e.g. a widget, karax based or not, I know of captured variables requiring effort, not sure if there is other stuff).

Most of the complexity is gone now thanks to the removal of all things gensym. So this is basically what we are doing now. Except that we are baking all the scripts into a single file (for ordinary nbJsFromCode) separated by blocks. This should improve build-times compared to compiling each nbJsFromCode as its own file (remember how slow cornerImage was in the nimConfSlide? It was exactly for this reason).
And the old complexity came basically solely from the colliding identifiers problem. So now that we have solved it (bumpGensym + blocks), it's not needed anymore. In theory, we could just use bumpGensym and keep the old behavior that all code blocks are totally isolated from each other and compile them one by one as you suggest. But the blocks are simple enough and allow us to have interop between code blocks, so I think it is the superior way forward.

Capturing variables is probably the most complex part that we have left, but it is "only" ordinary typed macro stuff to generate the correct code to serialize and deserialize the variable on the different sides.

[pietroppeter]

added some comments (mostly questions) from what I understand on the changes. Given that I did not understand the code you are replacing I will also add more questions in general reviewing the complete files in your actual branch

[pietroppeter]

Ok, I have read the rest and I can go on with comments and questions, it seems I understand now most of the code except the stuff in nimToJsString. Correct me on the statements below if I am wrong.

In nimib.nim we define basically 4 api to add js code generated from nim code:

• jsFromString: consists of 4 templates (nbJsFromStringInit, nbAddStringToJs, nbAddToDocAsJs and nbJsFromString). This is the easiest to understand as it does what one would guess: put the content in a nim file, compile it to javascript and then add it in the document in the place where the block was placed. The only piece of complexity here is the bumpGenSym that we are doing and we will do for every nim own file that compiles to javascript. This is though an api that I think is never used in the documentation (correct?) as you would need to put nim code in a string which is kind of ugly. There is no capture of variables (you can do it manually by interpolating strings). Not sure where this api should be used in place of the others but it is a sort of escape hatch in case the other have bugs for some reasons...
• jsFromCodeOwnFile: this is a first extension of the one before, you are now able to write nim code without putting it into a string AND you are also able to capture variables. Being an OwnFile you do the bumpGensym to avoid collisions of identifiers.
• nbKaraxCode: this the Karax api, build on jsFromCodeOwnFile without the boilerplate (and adding the Karax id for multiple Karax blocks in the same file). It also captures variables.
• nbJsFromCode: this is the one that uses the new intuition of a global js file. It assumes we have a single global script for every document. You can add stuff in the global scope or you can add stuff in a block scope. Great advantage is that all code in here gets compiled in a single script (so less time to compile and less duplicate js at the end). We are capturing variables of course. We do not actually add a block until there very end, currently this is put inside nbSave. Some thoughts about changing stuff here (besides the comments already done in the in code review):
  • what if instead of of nbJsFromCodeGlobal and nbJsFromCode we call them respectively nbAddJsFromCode and nbAddJsFromCodeInBlock? we are indeed adding js to a global script that has been initialized (as empty) during nbInit.
  • it is a bit weird that the block is added during nbSave. already in the in-code-review I propose to extract a proc. I would propose that we call it addJsGlobalCodeBlock or something similar. it could be called by the user as nb.addJsGlobalCodeBlock. Indeed we probably might want to add another field to NbDoc: isJsGlobalCodeBlockAdded: bool. So that in nbSave we check it and we add it only if it is false.
  • in this way we can add more html after this js code block (not sure if it would matter or not). But in particular something that we can do is to show the original code for js global code block with nbCodeToJsShowSource (should this template and the name of the command should be renamed from nbCodeToJs to nbJsFromCode? or maybe it is better like this since this block is actually shared also with the first api fromString?). I think currently we are losing the originalCode in nbJsFromCode blocks, we probably could add a new field to NbDoc to track "original" code.

On capturing variables:

• it seems that nimToJsString is the only big piece of complexity in jsutils (probably we could move compileNimToJs from renders into jsutils?), since nbKaraxCodeBackend is rather transparent. You confirm that this complexity is only for the captured variables? if this is the case could we in principle check with a when len(args) == 1 (directly in nbFromCode blocks) and NOT call nimToJsString?
• regarding capturing variables and nimToJsString could you explain better what that code does? probably it would help if you show the original vs transformed nim code to see how it would look. The code is really complex with two stages, a cache, a call to degensym...

Finally I think, having understood a bit more, we probably could improve the interactivity document:

• the first example we show should not probably be one with a captured variable
• we should mention the multiple apis and what are the sources of complexity, which should be essentially two: nim gensyms identifiers when compiling to js, we want to capture variables.
• regarding the multiple api the global js script api it seems to me is a novel and smart idea on how to solve the sources of complexity, so it should be given a bit more appreciation.
• also on capturing variable it is now evident to me (especially looking at the examples of two counters) that it is important for passing ids and have reusable widgets that contain both rawHtml and Js code. At first I thought its main use case was to pass complex objects generated from C, but I guess that is an additional (cool) use case (on which I do not think I have really seen examples, I mean in principle we could train a neural network with arraymancer and translating to a easier structure where we could do the inference on js... you know just to start with simple ideas :D).

[pietroppeter]

Ok, with the in-code-review and last comment, I think I have exhausted my review on current code. Hey I am taking this literally 🤣:

Keep shooting questions at me, I'm not merging this until you understand it ;)

[HugoGranstrom]

jsFromString...
This is though an api that I think is never used in the documentation (correct?) as you would need to put nim code in a string which is kind of ugly. There is no capture of variables (you can do it manually by interpolating strings). Not sure where this api should be used in place of the others but it is a sort of escape hatch in case the other have bugs for some reasons...

Correct observations! 👍 It shouldn't be used unless you have no other choice, basically. The implementation is so simple though that it doesn't hurt to keep it around.

jsFromCodeOwnFile...

Correct 👍

nbKaraxCode...

Correct 👍

nbJsFromCode

Correct observations 👍

what if instead of of nbJsFromCodeGlobal and nbJsFromCode we call them respectively nbAddJsFromCode and nbAddJsFromCodeInBlock? we are indeed adding js to a global script that has been initialized (as empty) during nbInit.

It entirely depends on what we decide on in the discussion of where to place the global blocks. If they are placed at the top, these names are misleading in my opinion. If they are mixed between each other, these names make more sense but are a bit too verbose imo. Better to remove the "add" then and go with nbJsFromCode and nbJsFromCodeInBlock. I think that the most recommended one (the block version) should have the shortest name, though...

it is a bit weird that the block is added during nbSave. already in the in-code-review I propose to extract a proc. I would propose that we call it addJsGlobalCodeBlock or something similar. it could be called by the user as nb.addJsGlobalCodeBlock. Indeed we probably might want to add another field to NbDoc: isJsGlobalCodeBlockAdded: bool. So that in nbSave we check it and we add it only if it is false.

It is common to put your javascript at the bottom of your body after all, so why not put it at the bottom of our page as well. I don't see any reason anyone would want to insert this anywhere else (there might still be reasons of course but ownFile covers those use-cases). Also, what if we run nbJsFromCode after we have saved the global script? Should it start on a blank new script or continue on the old one?

in this way we can add more html after this js code block (not sure if it would matter or not). But in particular something that we can do is to show the original code for js global code block with nbCodeToJsShowSource (should this template and the name of the command should be renamed from nbCodeToJs to nbJsFromCode? or maybe it is better like this since this block is actually shared also with the first api fromString?). I think currently we are losing the originalCode in nbJsFromCode blocks, we probably could add a new field to NbDoc to track "original" code.

I don't know what showing the original code for the entire global script would give us tbh. There would be multiple variables named the same for example, so it wouldn't be compilable Nim code. And the processed global script file isn't perhaps the prettiest of code, but could be good to have for debugging purposes. Here we could just add a new command which prints out the current code in nb.nbJsGlobalScript & nb.nbJsScript.
Instead, if you want to show the code of a single block, we should add something like this:

template nbJsFromCodeShow([args](args: varargs[untyped])) =
  nbCodeDontRun: # show the body of the code
    args[0]
  nbJsFromCode(args)

This way we don't have to save the original code.

[HugoGranstrom]

(probably we could move compileNimToJs from renders into jsutils?

Sure, will do 👍

You confirm that this complexity is only for the captured variables? if this is the case could we in principle check with a when len(args) == 1 (directly in nbFromCode blocks) and NOT call nimToJsString?

The complexity is mainly due to the capturing of variables, but there still is logic in there that we need all the time. We still need the degensym to clean varName`gensym32 and the logic to put the code inside of a block. So I don't see any way of avoiding calling nimToJsString. If we really wanted to, we could perhaps make it so it didn't have to call the second stage macro in these cases, but I don't see why we would do that. It would only make the code more fragmented and harder to follow.

regarding capturing variables and nimToJsString could you explain better what that code does? probably it would help if you show the original vs transformed nim code to see how it would look. The code is really complex with two stages, a cache, a call to degensym...

The fact that we have two stages is that we want the body to be untyped. This is because typed code has been transformed and isn't the code that we wrote originally. Here's an example:

# untyped
let a = 1
let b = 2
echo a + b

# typed
let a = 1
let b = 2
echo [a + b]

As you can see, if we compiled the typed code in a file, it would echo out an array instead of an int. So having the body be untyped is very important. But we also want to get the type of the variables we want to capture, so that we can unserialize them to the correct type on the javascript side. So we need a typed macro to get those types. The question that is, how do we pass the untyped body to a typed macro without making it typed? The answer is putting it in a cache table! In the typed macro (second stage), we can then get the untyped AST of the body from the cache table. The key to the element in the table, key: static string, is passed as an argument to the typed macro.

So this should hopefully bring some light on why we need the two stages, the cache table and degensym.

[HugoGranstrom]

Now for the actual capturing of variables!

let captureVars = toSeq(captureVars)

Here we save the symbols to the captured variables in a seq. Next we get the type of each symbol:

let captureTypes = collect:
  for cap in captureVars:
    cap.getTypeInst

Here we fetch the untyped body from the cache:

var body: NimNode
if key in bodyCache:
  body = bodyCache[key]

---

Next up we generate the assignments and deserialization of the captured variables:

# 1. Generate the captured variable assignments and return placeholders
let (capAssignments, placeholders) = genCapturedAssignment(captureVars, captureTypes)

If we for example have this code:

var x: int = 123
var y: string = "hello world"
nbJsFromCode(x, y):
  echo x, y

What genCapturedAssignment will give us is the following code:

let x = parseJson(placeholder`gensym1).to(int)
let y = parseJson(placeholder`gensym2).to(string)

and the list of placeholder-symbols it has generated. Now what's up with placeholder`gensym you might ask? We don't know the value of the variables at compile-time, so we create placeholders which we can then replace with the serialized values at run-time.

---

Next up we combine these new assignments and the body, degensym it, put it in a block (if specified) and turn the code into a string:

# 2. Stringify code
var code = newStmtList(capAssignments, body).copyNimTree()
code.degensymAst()

if putCodeInBlock:
  code = newBlockStmt(code)
var codeText = code.toStrLit

So We will basically have this string in the end:

let codeText = """
let x = parseJson(placeholder`gensym1).to(int)
let y = parseJson(placeholder`gensym2).to(string)
echo x, y
"""

---

Now we have come to the most complex part of it all: generating the replacements of the placeholders with the serialized json. We start off with assigning the codeString to an actual variable:

let codeTextIdent = genSym(NimSymKind.nskVar, ident="codeText")
result = newStmtList()
result.add newVarStmt(codeTextIdent, codeText)

So this will generate the following code:

var codeText = """
let x = parseJson(placeholder`gensym1).to(int)
let y = parseJson(placeholder`gensym2).to(string)
echo x, y
"""

Then we loop over each of the capture variables:

for i in 0 .. captureVars.high:

and for each one we do the following:

# Get string literal of the placeholder (e.g. "placeholder`gensym1")
let placeholder = placeholders[i].repr.newLit
# Get the variable symbol
let varIdent = captureVars[i]
# Generate the call to serialize the variable
let serializedValue = quote do:
  $(toJson(`varIdent`))

So the serialized value is basically obtained like this:

$(toJson(x))

The last thing we do in the loop is to do the actual string replacement of the placeholder with the json:

result.add quote do:
  `codeTextIdent` = `codeTextIdent`.replace(`placeholder`, "\"\"\"" & `serializedValue` & "\"\"\"")

This basically generates this code:

codeText = codeText.replace("placeholder`gensym1", &"{$(toJson(x))}")

And that is basically it. This final code on the C-side is:

var codeText = """
let x = parseJson(placeholder`gensym1).to(int)
let y = parseJson(placeholder`gensym2).to(string)
echo x, y
"""
codeText = codeText.replace("placeholder`gensym1", &"{$(toJson(x))}")
codeText = codeText.replace("placeholder`gensym2", &"{$(toJson(y))}")

[HugoGranstrom]

the first example we show should not probably be one with a captured variable

Good point 👍

we should mention the multiple apis and what are the sources of complexity, which should be essentially two: nim gensyms identifiers when compiling to js, we want to capture variables.

WIll add it to the Internal Workings section 👍

regarding the multiple api the global js script api it seems to me is a novel and smart idea on how to solve the sources of complexity, so it should be given a bit more appreciation.

Do you have any concrete examples in mind? (I don't 🤣 )

also on capturing variable it is now evident to me (especially looking at the examples of two counters) that it is important for passing ids and have reusable widgets that contain both rawHtml and Js code. At first I thought its main use case was to pass complex objects generated from C, but I guess that is an additional (cool) use case (on which I do not think I have really seen examples, I mean in principle we could train a neural network with arraymancer and translating to a easier structure where we could do the inference on js... you know just to start with simple ideas :D).

Haha yes, it has many use-cases :) I haven't tried it out with any complex datatypes yet tbh, might be a good excuse to test its limits and do something cool with it as you say 😎 In your planned Machine learning tutorial on SciNim for example, if we trained some simple machine learning model and provided an interactive way of testing it using nbJs somehow. Basically training it in the C-side and making predictions on the JS side with user inputs.

[HugoGranstrom]

Ok, with the in-code-review and last comment, I think I have exhausted my review on current code. Hey I am taking this literally rofl:

Haha that's good! 😄 Didn't expect to sit nearly 2h answering 🤣 But that time will be well spent if it means I can save more hours in the future now that you know how this all works and can cover for me ;)

[pietroppeter]

Thanks a lot for the answer and the great write up!

I have been reading and thinking about this stuff, and I have had some interesting thoughts (some outside of scope of this PR). Before doing that, I will like first to list stuff that is undecided, to do or needs more discussion:

from in-code review:

• first 4 remarks are action to be taken, small details, when we close them we resolve the conversation
• decision to take: do we interleave global scope and blocks or not?
• change logic of bump gensym, likely adding a counter and looping inside the macro. 👍

rest of conversation.

• decision to take: do we rename nbJsFromCodeGlobal and nbJsFromCode to nbJsFromCode and nbJsFromCodeInBlock (agree that we should skip add)?
• decision to take: should we allow to add a final js block outside of nbSave?
• decision to take: should we do something about possibility of showing code in JsFromCode?
• move compileNimToJs from renders.nim to jsutils.nim
• light on why we need the two stages, the cache table and degensym
  • the explanation looks very convincing to me, still need to understand details, will do it on my own time trying stuff when I am in the mood
• explanation of variable captures
  • again here good explanation, thanks, will need to work more on it on my own
• first example will not be with capture variables
• will add a internal working sections on sources of complexity
• not really have concrete example on how to show appreciation about single js script with global and block sections.

first off, did I miss something? I intentionally did not quote here, but I tried to go with order of appearance.

[pietroppeter]

looks good. just add a few suggestion to align block names to new naming convention.
I may have lost some nbJsToCode along the way (especially if they do not appear in the diff..).

Will now look again at the preview docs but I guess we are basically done! :)

[pietroppeter]

Had a look at the preview docs and had a few more remarks:

• first JsFromCode code is not shown in the document but it should
• we could mention at the end of nbKaraxCode section of interactivity.nim that we have another example of using nbKaraxCode in caesar
  • in index.nim short description of caesar we refer to it as being built with nbJsFromCode while it uses nbKaraxCode

I think with this I finally completed the review. Sorry for the delay and again great work on this!

[HugoGranstrom]

first JsFromCode code is not shown in the document but it should

Not sure I understand what you mean :/

we could mention at the end of nbKaraxCode section of interactivity.nim that we have another example of using nbKaraxCode in caesar
in index.nim short description of caesar we refer to it as being built with nbJsFromCode while it uses nbKaraxCode

Will fix!

[HugoGranstrom]

first JsFromCode code is not shown in the document but it should

Now I found it. Changed nbCode to nimibCode and it worked.

I think with this I finally completed the review. Sorry for the delay and again great work on this!

🥳
No worries 😄 Hopefully I shouldn't have to update the nbJs stuff in a while now that it's so simple, so the question is what I should do now 🤣

[pietroppeter]

the question is what I should do now

well, there is plenty of open issues... 😆

[pietroppeter]

ready to merge, right?
I guess after merging we should also bump version and release, right?
if you want to make the preparations (bump version, finalize changelog for release, ...) in this PR, otherwise I'll do it later

[HugoGranstrom]

🤣 very true, 51 open issues should have something yummy in them.

I can bump the version and changelog when I get home 👍 I think this is my first time making a new release on my own? 😯

[pietroppeter]

I think this is my first time making a new release on my own? 😯

then go ahead and enjoy the release! 🚀

[HugoGranstrom]

Versions are bumped, hope I didn't forget anything now...
Will have a look through the preview before merging this.

[pietroppeter]

ah, maybe add a line about the new nimibCode in changelog?

[HugoGranstrom]

Good point :+1 Looked through the docs before pushing the last commit and everything seems to work fine. So I'll merge this after dinner unless we find something else until then.

[pietroppeter]

looks good to me.

the more I think about it the more stuff it pops to mind. we probably should link the NimConf 2022 presentation (and slides) in the readme/index? not sure in the minimal way we did for 2021 or showing the video thumbnail (would look more attractive). to be honest this could all be separate PRs but since we are here...

[HugoGranstrom]

It would be nice with a thumbnail, but we'd just have to get a better thumbnail where we both are in it. I have uploaded one such example. But I think it is better if we merge this first so we can get a static link to it in the main branch instead before linking to it. So I'll merge this and create a follow-up PR for that.