dthree / vorpal

Node's framework for interactive CLIs
http://vorpal.js.org
MIT License
5.64k stars 280 forks source link

Reference Error: ui is not defined #96

Closed shayne closed 8 years ago

shayne commented 8 years ago

As of vorpal 1.8.8 I'm getting the following when issuing vorpal.ui.redraw.clear(); vorpal.ui.redraw.done()

Unhandled rejection ReferenceError: ui is not defined
    at Function.UI.redraw.clear (/Users/sweeney/Code/vorpal-trello/node_modules/vorpal/dist/ui.js:628:10)
    at _callee2$ (fetcher.js:27:22)
    at tryCatch (/Users/sweeney/Code/vorpal-trello/node_modules/babel-regenerator-runtime/runtime.js:61:40)
scotthovestadt commented 8 years ago

Looking into it.

dthree commented 8 years ago

Thanks.

dthree commented 8 years ago

Yes - oh that just means @shayne is using the wrong command I guess.


@shayne check out this Wiki to ensure you're using the syntax properly:

https://github.com/dthree/vorpal/wiki/api-|-vorpal.ui#uiredrawtext-text

scotthovestadt commented 8 years ago

The issue is that those methods are on UI.redraw.clear and UI.redraw.done, not actually on the UI class. Since the UI is a singleton we can inject the reference to it but it's just weird design that wasn't caught when we moved to Babel. I'm working on it, seeing if it's possible to put together a test case.

dthree commented 8 years ago

? Not sure what moving to Babel does to this?

scotthovestadt commented 8 years ago

@dthree It's because UI wasn't a class before, it was just an object, so the redraw function is a little weird. I think I've submitted a fix (please review). I'd like to get a test case for this type of thing too.

dthree commented 8 years ago

Okay - looks okay. Could you look at this though:

https://github.com/dthree/vorpal/blob/e96e37daf2b8886155f219d83639a013a1a1f314/lib/ui.js#L569

UI already has some... interesting functionality in that it has to extend global for a single instance. This is really important, so I want to make sure these changes don't mess with that.

scotthovestadt commented 8 years ago

Yeah, I looked at this earlier, I think it's basically a singleton. It's interesting design but I don't see an issue with it.

dthree commented 8 years ago

Okay. Go ahead and merge, and I'll publish. :+1:

scotthovestadt commented 8 years ago

Should be ready for publish but if @shayne can verify it solves his issue that'd be ideal.

dthree commented 8 years ago

Okay we'll wait up a bit for him. I think I stumbled into this as well yesterday but didn't have time to look into it, so I'll check it out on my pjt.

dthree commented 8 years ago

Okay I pretty thoroughly tested it. Published, but want to wait for @shayne before closing this issue.

Thanks for your help @scotthovestadt - I'm happy Vorpal has a bus factor of 2 now :smiley:

shayne commented 8 years ago

This works great. Amazing speed. :trophy:

dthree commented 8 years ago

Great!