lukeed / taskr

A fast, concurrency-focused task automation tool.
MIT License
2.53k stars 74 forks source link

Not informative message when plugin not presented #195

Closed hzlmn closed 8 years ago

hzlmn commented 8 years ago

Here I forgot to install fly-babel preset and it throws ugly exception instead of message like fly-babel plugin is not presented

➜  xeon-bash-ast git:(master) ✗ fly  
[12:2:40] Flying with /home/olehkuchuk/Projects/xeon-bash-ast/flyfile.js...
[12:2:40] Starting default
[12:2:40] Watching files...
[12:2:40] Starting babel
stack: 
  - TypeError: this.source(...).babel is not a function
  -     at Fly.x.babel (~/Projects/xeon-bash-ast/flyfile.js 9:6)
  -     at next (native)
  -     at onFulfilled (~/Projects/flyproject/node_modules/co/index.js 65:19)
  -     at ~/Projects/flyproject/node_modules/co/index.js 54:5
  -     at Fly.co (~/Projects/flyproject/node_modules/co/index.js 50:10)
  -     at Fly.toPromise (~/Projects/flyproject/node_modules/co/index.js 118:63)
  -     at next (~/Projects/flyproject/node_modules/co/index.js 99:29)
  -     at onFulfilled (~/Projects/flyproject/node_modules/co/index.js 69:7)
  -     at ~/Projects/flyproject/node_modules/co/index.js 54:5
  -     at Fly.co (~/Projects/flyproject/node_modules/co/index.js 50:10)
message: this.source(...).babel is not a function
[12:2:40] babel failed due to TypeError: this.source(...).babel is not a function
[12:2:40] Finished default in 31 ms

@lukeed @bucaran

lukeed commented 8 years ago

Closing only because it's a duplicate of #145. Our error messages are sub par.

hzlmn commented 8 years ago

@lukeed btw, I am working on this.

The major problem is that when Fly loads plugins it bounds that to original fly instance and there is no identifier for something like is it a plugin or just a fly method.

There are few solutions to this problem. I will submit PR today or tomorrow.

hzlmn commented 8 years ago

@lukeed

Also, I was wondering how critical is rewritting plugin API could be?

Basically providing good error messages in this scope could be very tricky. Also there are some potential problems with binding this to plugin instance, maybe it's better to share context via dependency injection.

It wont change API much, like

  module.exports = (fly) => 
      fly.filter('whatever' , callback)

but here we have isolated fly instance that wont point to main fly instance

What do you think :question:

lukeed commented 8 years ago

@hzlmn I'm not sure I see what you gain from doing this? (I also don't have code in front of me)

hzlmn commented 8 years ago

@lukeed isolation of fly instance, for example here sample plugin api (structure can be redefined)

  module.exports = (fly) =>
     fly.plugin({
       type: fly.filter,
       handler: whateverFn
     })

fly is a shallow copy with binded method plugin that have reference to real this. In this case we are sure that fly instance readonly and if someone want to redefine for example self.root, it will be only in scope of plugin.

btw, its just food for thoughts

ghost commented 8 years ago

@hzlmn I remember @TrySound saying something similar to this when the project first got started. I don't know if he'd like to comment on it or discuss the idea with you.