Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Losing reference to module when using lodash=_ #22

Open
shockey opened this issue Aug 25, 2016 · 4 comments
Open

Losing reference to module when using lodash=_ #22

shockey opened this issue Aug 25, 2016 · 4 comments

Comments

@shockey
Copy link
Collaborator

shockey commented Aug 25, 2016

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
Copy link
Owner

victorb commented Aug 25, 2016

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
Copy link
Collaborator Author

shockey commented Aug 25, 2016

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
Copy link
Collaborator Author

shockey commented Aug 25, 2016

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
Copy link
Owner

victorb commented Jan 21, 2017

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants