[DISCUSS] option for ignoring const and let

[slavaGanzin]

Hello, Nicolas.

May I suggest to add argument to kernel which cut const and let keywords before processing code. In hydrogen it's really annoying to get: Identifier is already declared while you experimenting with code.

I can make PR

[n-riesco]

Here are some quick thoughts:

Should we open this issue in node's repository?

I guess, what you have in mind is to have a flag so that const and let expressions are replaced by var expressions (only if they appear in the global scope).

Since IJavascript is providing access to node REPL sessions, I think the place to implement this change is in node's REPL. See:

$ node
> const x=1
undefined
> const x=2
TypeError: Identifier 'x' has already been declared
    at repl:1:1
    at REPLServer.defaultEval (repl.js:252:27)
    at bound (domain.js:287:14)
    at REPLServer.runBound [as eval] (domain.js:300:12)
    at REPLServer.<anonymous> (repl.js:417:12)
    at emitOne (events.js:82:20)
    at REPLServer.emit (events.js:169:7)
    at REPLServer.Interface._onLine (readline.js:210:10)
    at REPLServer.Interface._line (readline.js:549:8)
    at REPLServer.Interface._ttyWrite (readline.js:826:14)

If not, should we use a parser?

What did you have in mind for a PR?

I reckon a PR like this would require a Javascript parser. I think a parser would be too heavy a dependency for just an optional feature.

How about IJavascript introducing optional plugins?

I'm thinking of a flag similar to the current --ijs-startup-script=path; something like --ijs-plugin=plugin, where plugin would be the name of an npm module that exports things like the transpile function used by jp-kernel.

[n-riesco]

Some difficulties:

• let process; https://github.com/nodejs/node/issues/6802

---

And some notes to myself:

• https://github.com/nodejs/node/issues/855#issuecomment-74583712

• https://github.com/nodejs/node/issues/8810

[slavaGanzin]

IMHO. It's hard to communicate with Node's core. Because "they knows better"

Parser would be an overkill.

Adding plugins is a great idea. But they maybe an overkill too.

As an idea:

--ijs-transform-in='in => in.replace(/\bconst\b/, '')'
--ijs-transform-out='require("some-plugin").transform'

I think const/let elimination should be experimental feature. If you use it you should be aware of consequences. "Provided as is" And I think regex like ^const|let\s+\w+\s*= would be acceptable heuristics. Not the best one, but acceptable

[n-riesco]

What would be the difference between --ijs-transform-in and --ijs-transform-out? You'd like to transform the output MIME bundle?

[slavaGanzin]

You'd like to transform the output MIME bundle?

MIME... It would be to complicated. Let's forget this

[ericuldall]

This definitely doesn't behave as expected. I would expect it to run similar to calling node test.js each time I run my script. I'm currently working with this kernel in Hydrogen for the atom editor.

Is there some way to run the script like new, clearing previous memory, when I make changes to variables?

[rgbkrk]

You have to use "Hydrogen: Restart Kernel" to restart your session. There is another builtin command in Hydrogen called "Hydrogen: Restart Kernel and Re Evaluate Bubbles" which will restart and then run each of the lines or blocks you've done previously as individuals chunks of code. At least for me, I have these two in my keymap:

'atom-text-editor':
   'cmd-enter': 'hydrogen:run'
   'cmd-b': 'hydrogen:restart-kernel-and-re-evaluate-bubbles'

[Bamieh]

I would solve this issue like this:

1. replace the following line of code:

ContextifyScript.Script.runInThisContext();

with

ContextifyScript.Script.runInContext(currentContext);

2. Check for first error

If the code errors out, parse the error to check for this pattern:

SyntaxError: Identifier '*' has already been declared

if this pattern matches, create a new context to be used across the current session, and run the code again, any error created afterwards is propagated to the user.

function createContext() {
  const sandbox = {
    Buffer,
    clearImmediate,
    clearInterval,
    clearTimeout,
    setImmediate,
    setInterval,
    setTimeout,
    console,
    process,
    // ...other ijavascript-y needed context
  };
  sandbox.global = sandbox;
  return sandbox;
}

try {
    ContextifyScript.Script.runInContext(currentContext);
} catch(e) {
   if(DOUBLE_DECL_PATTERN.test(e.message)) {
      currentContext = vm.createContext(createContext());
      return ContextifyScript.Script.runInContext(currentContext);
   }
   throw e
}

I have not looked into the ijavascript code yet, but I am hoping to get some feedback on this before I do so.

[n-riesco]

@Bamieh At the very beginning of IJavascript, I experimented with runInContext, but I ruled it out because this approach is riddled with issues, e.g.:

[] instanceof Array // true

vm.runInContext("[] instanceof Array", vm.createContext({Array})); // false

---

Another difficulty with this idea of recreating the context is that the state of previous context would be lost.

---

The way I see this issue is that IJavascript is an enriched REPL for Node.js and it aims at behaving the same way Node.js's REPL does.

When I use IJavascript as a REPL, I prefer to use var to define variables in the global context, and let and const inside functions.

When I use IJavascript to run a a script, then the normal rules for const and let apply, and this isn't an issue.

---

Picking up @rgbkrk 's suggestion and your idea, I could implement $$.restart() to trigger a kernel restart (so that the user wouldn't have to do the same by other means).

[Bamieh]

@n-riesco why would you pass Array to the new context 🤔 , it gives false because the Array you passed is pointing to a different location in memory than the one created by [] since both run in a different v8 contexts. By not passing Array, this will evaluate to true as it should. The createContext I provided is valid for production uses.

To solve the previous context issue, we just track added objects on the global (observers or manually) and propagate them in the new context.

---

Implementing a restart is not a good idea imo, since it might break other code blocks depending on previous code. If the code is to be run in an isolated scope, anyone can write this:

{
const ..
}

blocks_are_the_new_iffe 😆

[n-riesco]

@Bamieh I'm moving the discussion to https://github.com/n-riesco/nel/issues/9