Support code hot reloading for JavaScript projects #7362

zah · 2018-03-19T00:24:59Z

Please see karaxnim/karax#55 for a working example of this.
More details can be found in the updated compiler guide.

…oesn't follow NEP-1

zah · 2018-03-19T16:17:45Z

One of the Travis builds has timed-out. Seems to be a false-negative.

dom96 · 2018-03-22T18:16:36Z

lib/js/dom.nim

    once*: bool
    passive*: bool

+  MathLib* = ref object


The functions and types you added here aren't part of the DOM. They should be moved to a different module.

Not if you treat the DOM as the larger API surface of the browser. What would be a suitable name for this other module?

In Javascript there is the Core (that is, the part of the stdlib that is requested in all environments, even outside browers), the DOM and the BOM (Browser object model). https://vkanakaraj.wordpress.com/2009/12/18/javascript-vs-dom-vs-bom-relationship-explained/

Objects such as Math, Date and so on live in the code JS library

jscore then?

Could make sense, although I expect most of this functionality to be available in the usual parts of the stdlib with the usual Nim names (e.g. under the modules math or times or random)

The point of these exposed APIs is that they allow you to call the "low-level" native methods of the JavaScript environment from jsffi-based code, which adds only a very thin layer on top of JavaScript.

Nim modules such as math and times have a slightly different goal. For all intents and purposes, they try to preserve the exact behavior of the C run-time of Nim, which often means having a thicker layer responsible for converting between the Nim and JavaScript data types and formats.

Agreed. I would use jscore, dom and bom (the latter for things such as navigator, history etc. if we have it)

dom96 · 2018-03-22T18:17:47Z

lib/js/jsffi.nim

  ## Checks, whether `x` has a property of name `prop`.

-proc jsTypeOf*(x: JsObject): cstring {. importcpp: "typeof(#)" .}
+proc jstypeof*(x: JsObject): cstring {. importcpp: "typeof(#)" .}


camelCase is valid for this case, not sure why you've changed it.

I made everything consistent. The other definitions were following this scheme, but I agree that the whole module could have followed NEP-1 from the start.

dom96 · 2018-03-22T18:19:22Z

lib/js/jsffi.nim


 template toJs*(s: string): JsObject = cstring(s).toJs

+template `$`*(o: JsObject): cstring = cast[cstring](o.toString())


Where is toString defined? What does it return, and why is it safe to cast to a cstring?

o is a JsObject. toString gets compiled with the magic of the dot syntax.

This doesn't return string but cstring and so does not adhere to Nim's stdlib. Change its name to toJsString or similar.

dom96 · 2018-03-22T18:20:30Z

lib/system.nim


+template once*(body: untyped): untyped =
+  ## Executes a block of code only once (the first time the block is reached).
+  ## When code hot reloading is enabled, protects top-level code from being


Minor nitpick but please s/code hot reloading/hot code reloading/

Hrm, you are right after googlefighting the terms. I'll change that.

dom96 · 2018-03-22T18:21:19Z

lib/js/jsffi.nim

+  jsdirname* {.importc: "__dirname", nodecl.}: cstring
+    ## JavaScript's __dirname pseudo-variable
+  jsfilename* {.importc: "__filename", nodecl.}: cstring
+    ## JavaScript's __filename pseudo-variable


I'd say all of the above variable names should be camel cased.

dom96 · 2018-03-22T18:22:49Z

lib/js/jsffi.nim

+
  NotString = concept c
    c isnot string
-  js* = JsObject


Removing js and JsRoot will cause breakage. Is this intentional? Doesn't this module depend on the definition of js?

This was requested by Araq.

Where was this requested by Araq? On IRC? Slack?

I just asked him about it via PM and he doesn't really seem sure of the reason for this request.

It was my impression that this shortcut alias was introduced in this PR secretly. My bad. Still I don't like this and we might want to deprecate it but that's not the point of this PR. Sorry! Please re-add it.

It was added by myself not so long ago with other fixes and additions to jsffi. It's quite likely that our own project is the only user of this short-cut.

That makes more sense. A separate PR for these things would be appreciated. The more unrelated changes are made in a single PR the harder it will be for us to figure out why the changes have been made in the future.

I thought this js global has been in jsffi since its initial inclusion. If it's something that was quickly added in a previous PR then it might be good to remove it, globals with such common names are usually a bad idea.

We are using the shortcut quite often too :(, altho nowadays we added converters to eliminate .to almost completely its still useful for js{} etc!

I love this short-cut in our own code too, but Araq's argument is that it violates NEP-1 and it's easy to introduce in a project with a simple:

type js = JsObject

Can't we just use type Js = JsObject then to be totally NEP-1 compatible

dom96 · 2018-03-22T18:26:23Z

doc/nimc.rst

+.. code-block:: Nim
+  var settings = initTable[string, string]()
+
+  once:


Wouldn't it make more sense to do it the other way around? It seems like most code will have to be under this once invokation, but maybe I'm wrong.

I think you are wrong, but we'll see. We already use the hot reloading branch in a quite complex project and so far this policy makes more sense.

dom96 · 2018-03-23T17:24:38Z

I documented the jscore module, hope you don't mind :)

alehander92 · 2018-03-23T19:57:12Z

Related to jscore module.

I have some local node.js/browser API type definitions(similar to jscore json) and I planned to map some of them to nim stdlib methods: does this sound useful, and where would be the best place to put them? near the existing definitions in when defined(node):, in new js lib files, something else?

Araq · 2018-03-24T10:43:12Z

I don't understand why once is required, seems to break the abstraction.
once is nothing special, what is done here is that .global vars have run-once semantics, if I'm understanding this correctly.

alehander92 · 2018-04-07T06:57:26Z

Is there anything blocking that? I am basing my sourcemap #7508 on that branch currently

zah added 3 commits March 19, 2018 13:36

Support code hot reloading for JavaScript projects

3094ed3

Add some missing JavaScript symbols and APIs

9fdd300

fix the Travis build

31deb0a

zah force-pushed the js-hot-reloading branch from fdf6132 to 31deb0a Compare March 19, 2018 11:37

(review changes) remove the js type from the standard library as it d…

e9458f8

…oesn't follow NEP-1

more additions to the DOM module

443ab4b

dom96 reviewed Mar 22, 2018

View reviewed changes

zah and others added 3 commits March 23, 2018 12:50

Follow NEP-1 in jsffi; spell 'hot code reloading' correctly

980e30c

introduce a jscore module

42cda6b

Document jscore module.

a744520

Araq added 3 commits April 13, 2018 11:34

Merge branch 'devel' into js-hot-reloading

1ddaa65

readded js type

6bcded7

Remove the '$' operator that doesn't behave

6501813

Araq closed this Apr 13, 2018

Araq reopened this Apr 13, 2018

Araq merged commit e3037a2 into devel Apr 13, 2018

narimiran deleted the js-hot-reloading branch October 11, 2018 06:54


		template toJs*(s: string): JsObject = cstring(s).toJs

		template `$`*(o: JsObject): cstring = cast[cstring](o.toString())

Uh oh!

Support code hot reloading for JavaScript projects #7362

Support code hot reloading for JavaScript projects #7362

Uh oh!

Conversation

zah commented Mar 19, 2018

Uh oh!

zah commented Mar 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zah Mar 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zah Mar 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dom96 commented Mar 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alehander92 commented Mar 23, 2018

Uh oh!

Araq commented Mar 24, 2018

Uh oh!

alehander92 commented Apr 7, 2018

Uh oh!

Uh oh!

zah commented Mar 19, 2018 •

edited

Loading

zah Mar 23, 2018 •

edited

Loading

zah Mar 24, 2018 •

edited

Loading

dom96 commented Mar 23, 2018 •

edited

Loading