Losing reference to module when using `lodash=_`

shockey commented 8 years ago

trymodule@1.4.0 on node 4.2.1.

→ trymodule lodash=_
Gonna start a REPL with packages installed and loaded for you
'lodash' was already installed since before!
Package 'lodash' was loaded and assigned to '_' in the current scope
REPL started...
> _.each
[Function: forEach]
> _.each
undefined
> _
undefined
>

It may have something to do with using _ as the local variable.

Can someone try to reproduce this?

victorb commented 8 years ago

Yeah, I can reproduce this is as well. It seems like assigning via context does not work the same as assigning inside the repl instance. _ in the nodejs repl is usually used for storing the last results. So either, the assignment has to come after starting the repl somehow, or we can see this as a bug in the nodejs repl and try to file a bug.

See the following example:

➜  .trymodule trymodule lodash=_
Gonna start a REPL with packages installed and loaded for you
'lodash' was already installed since before!
Package 'lodash' was loaded and assigned to '_' in the current scope
REPL started...
> _.each
[Function: forEach]
> _.each
undefined
> _.each
TypeError: Cannot read property 'each' of undefined
    at repl:1:2
    at REPLServer.defaultEval (repl.js:272:27)
    at bound (domain.js:287:14)
    at REPLServer.runBound [as eval] (domain.js:300:12)
    at REPLServer.<anonymous> (repl.js:439:10)
    at emitOne (events.js:95:20)
    at REPLServer.emit (events.js:182:7)
    at REPLServer.Interface._onLine (readline.js:211:10)
    at REPLServer.Interface._line (readline.js:550:8)
    at REPLServer.Interface._ttyWrite (readline.js:827:14)
> "123"
'123'
> _
'123'

Tried to defining _ as read-only with the context of the nodejs repl but then every execution fails, since nodejs tries to assign _ the latest returned value... Not entirely sure here

shockey commented 8 years ago

Per the Node.js documentation:

Assignment of the _ (underscore) variable

The default evaluator will, by default, assign the result of the most recently evaluated expression to the special variable _ (underscore).

Explicitly setting _ to a value will disable this behavior.

(source)

We can file a bug. I'd like to see consistency between respecting assignment and vars provided via a context object.

However, since the REPL is in Node.js core, older and current versions will never get the fix... so with an eye towards our user experience, I think we should patch this on our side.

shockey commented 8 years ago

Potential fixes:

1. Programmatically assign `_`

The only way I can see to do this would be to wrap the REPL's eval() function with code that places our _ back into the REPL context.

2. Disallow `_` as a variable name

We can detect attempted use of _ as a variable name and fall back to using the module's name, and display a message explaining what happened.

victorb commented 7 years ago

Yeah, I'm leaning towards option 2 as I think it would work longer (considering that eval changes) and require less magic that people working in the repl might not be used to. Haven't really fully made up my mind though...

victorb / trymodule