Error with `nbCodeToJs` and code that needs to be executed exclusively on js backend

[pietroppeter]

example:

import nimib

nbInit
nbCodeToJs:
  import std/jsffi

trying to compile the above an error is produced:

~\Documents\GitHub\nblog\x_import_jsffi.nim(4, 1) template/generic instantiation of `nbCodeToJs` from here
~\Documents\GitHub\nimib\src\nimib.nim(145, 30) template/generic instantiation of `nbCodeToJsInit` from here
~\Documents\GitHub\nimib\src\nimib.nim(129, 43) template/generic instantiation of `nimToJsString` from here
~\Documents\GitHub\nimib\src\nimib\jsutils.nim(189, 26) template/generic instantiation of `checkIsValidCode` from here
~\.choosenim\toolchains\nim-1.6.6\lib\js\jsffi.nim(36, 10) Error: fatal error: Module jsFFI is designed to be used with the JavaScript backend.

indeed in jsffi there is a fatal error thrown if not running with js backend: https://github.com/nim-lang/Nim/blob/version-1-6/lib/js/jsffi.nim#L35

and this is the call that raises error in nimib: https://github.com/pietroppeter/nimib/blob/main/src/nimib/jsutils.nim#L189

thoughts on how to fix this @HugoGranstrom ?

[pietroppeter]

the PR linked above is a workaround but probably there is a better solution.

[HugoGranstrom]

Oh boy, that's a hard nut to crack :/ The entire reason we do the checkIsValidCode is to find out if it potentially could be a string instead of an untyped block of code. The best of course would be if we could capture this error but that feels unlikely.

An alternative workaround would be to auto-import all known JS-only modules similarly to how we insert import std/json here. Those would be inserted into the AST after the check so we should be fine. But even then we would have problems with external js-only libraries not working.

[HugoGranstrom]

Another workaround would be to inspect the code before the check, remove all imports, and then re-insert them again afterwards.

[pietroppeter]

The entire reason we do the checkIsValidCode is to find out if it potentially could be a string instead of an untyped block of code.

wait, is this the only reason? what we would do if it is indeed a string? For the string case could we just use an overload (of nbCodeToJs and related pieces) and implement a different logic there, so that the untyped overload will never see the string case?

[HugoGranstrom]

wait, is this the only reason? what we would do if it is indeed a string? For the string case could we just use an overload (of nbCodeToJs and related pieces) and implement a different logic there, so that the untyped overload will never see the string case?

If it is a string we use the string as the final code string, no magic insertions or anything. Eg:

nbCodeToJs: """
echo "Hello World"
"""

Hehe, we are back where we started now, the reason we couldn't have an overload was that it would have to type-check the argument to see if it was a string. So our body suddenly was typed instead of untyped, and as the code is invalid on its own, the compiler complained. So we would need separate names for the untyped and string case, but I think we went with this combined case as it was possible. Now in hindsight, we have almost exclusively used the untyped version so extracting the string version into let's say nbCodeToJsString would be a reasonable action to me. What do you think?

[pietroppeter]

Hehe, we are back where we started now, the reason we couldn't have an overload was that it would have to type-check the argument to see if it was a string. So our body suddenly was typed instead of untyped, and as the code is invalid on its own, the compiler complained. So we would need separate names for the untyped and string case, but I think we went with this combined case as it was possible. Now in hindsight, we have almost exclusively used the untyped version so extracting the string version into let's say nbCodeToJsString would be a reasonable action to me. What do you think?

ah, yes, back to square one :). Indeed, I did not recollect that we had a string version and we could not use the overload trick. Yes, I agree we could have a separate name for the string version, I am tempted to say maybe it could be nbStringToJs, but to be honest recently when trying to recall the api I am always starting with nbJs... so I think maybe better names for both could be nbJsFromCode and nbJsFromString. What do you think of this change (we would deprected nbCodeToJs)?

[HugoGranstrom]

I agree that the nbJs- prefix is good and makes more sense overall :+1: So I'm ok with that name-change :)

I'll spin up a PR with all our discussed changes tomorrow and start working on them then.