Add note about minimum Node.js version #16370
Conversation
Fix the error message
|
related #16054 |
| @@ -93,7 +93,10 @@ file. However, you can also run the code with `nodejs`:idx: | |||
|
|
|||
| nim js -d:nodejs -r examples/hallo.nim | |||
|
|
|||
| If you experience errors saying that ``globalThis`` is not defined, be | |||
There was a problem hiding this comment.
nit: better to write the error you got verbatim, makes it more likey a user will find this note:
ReferenceError: globalThis is not defined
There was a problem hiding this comment.
Yeah, you're right, but I feel that this message would still not be clear enough. The point is that the error pops up only if you include some libraries (e.g., random, time). However, the fact that Nim generates JS code that accesses globalThis means that Nim 1.4 and Nim devel are not being tested with versions of Node.js older than 14 (my guess that version 12+ is fine only comes from a quick read of the documentation of globalThis). It cannot be excluded that people using Node.js might experience other kinds of errors even when the generated JS does not contain globalThis, if the Node.js version they are using is not the same as the one used to test the Nim JS backend.
Do CI builds run tests using Node.js? If it is so, perhaps it is better to write explicitly in the documentation what is the minimum Node.js supported by Nim (12? 13? 14?).
There was a problem hiding this comment.
perhaps it is better to write explicitly in the documentation what is the minimum Node.js supported by Nim
I guess you just did with your PR, sort of. As mentioned, #16054 is related, to relax this requirement, but it's probably not worth it, updating node is easy enough IMO.
Fix the error message
Fix the error message
Add a warning about the minimum supported version of Node.js in the
docs. (See issue #16369.)