Request: Autoload Plugins w/Programmatic

[adamkiss]

When used programmatically, fly doesn't autoload & mount plugins, even though tasks are defined.

Minimal example: after creating new project, create following index.js and run node index.js

const Fly = require('Fly')
const fly = new Fly({
  tasks: {
    * default (fly){ yield fly.source('x.sass').sass().target('.') }
  }
})
fly.start()

Expected: new x.css file. Actual result: Unhandled rejection TypeError: fly.source(...).sass is not a function

I've also created minimal repo. Edit: Taken down since.

Node v6.9.1 LTS, npm v4.4.4, yarn v0.21.3

[hzlmn]

Hi! Yes, right now when you define your task with API you should manually pass plugins , that you gonna use Like so

const fly = new Fly({
  plugins: [
    require("fly-sass")
  ],
  tasks: {}
})

See more

[adamkiss]

@hzlmn Hi, thanks!

So… no automatic loading? In that case, I think the readme needs to be edited, because these parts (emphasis mine):

Passing plugins is also achievable; applicable for external and local plugins.

If you don't provide a file or a tasks object, your plugins will not be mounted to the instance.

read as if adding the modules was voluntary, e.g. if you have custom plugins/plugins that don't follow the fly-* convention.

Is the autoloading when programmatically using planned? Or?

Thank you.

[hzlmn]

As for me it's a correct behavior, but yes then readme should be changed. Need feedback from @lukeed is it a bug or request for improvements.

Anyway thanks for reporting it.

[adamkiss]

Nah, thank you for fly :)

---

Also, IMO, if plugins are not defined in the Fly constructor, they should be autoloaded.

Every system that supports/needs system-* plugins has at least one autoloading package, so sooner or later it will come anyway. But, fly already has the logic built in it, and it's tested and it's async and it's as good as it can be, I think.

From reading the code, it really seems that it's just a matter of rewriting this line to something like

const plugins = opts.plugins || plugins.load() || []

and requiring ../plugins.

Thank you for considering it.

[lukeed]

Hey hey,

Yeah this is intended behavior. The programmatic use case is one where the automatic behaviors from the CLI aren't wanted &/or don't suffice. So everything is available for customization & tberefore must be specified.

I'll make a note in the Readme for now (programmatic section).

But I'll have to think about your autoload suggestion some more. There's actually quite a few drawbacks in introducing some done-for-you behavior here.

Thanks!

[hzlmn]

@lukeed Maybe we could add it as an option preload or somethig, like in fly-babel? So it will be explicit.

[lukeed]

Yeah that's what I'm arriving at too.

But if plugins are preloaded, should additional plugins be allowed to pass thru the array? I'm thinking no.

[hzlmn]

Yep, think so too, there should be only single source of truth. Either manually passed plugins or preloaded.

[lukeed]

Cool, I'll hop on this in a minute or two.

[adamkiss]

@lukeed @hzlmn Hey, thanks for considering this!

Just wanted to add: maybe just exposing the already done internal method via Utils or something would be enough?

I mean, when I proposed the autoloading the plugins, I was considering mostly my use case - which I then realised is not something you should do with open source you want people to adopt.

Maybe just exposing it is even better?

Thank you anyway.

[lukeed]

I'm going to do:

const fly = new Fly ({
  plugins: true,
  tasks:{...}
})

So that no one has the ability to even try to do both.

[lukeed]

@adamkiss Thanks. Since the (auto-)loading of plugins is only done once & has to be done at a specific time, it wouldn't make sense here to expose them and/or let the user decide when to load. Doing so would be a much heftier refractor.

[adamkiss]

Damn, you guys are fast!

I didn't even realize you've reconsidered in the meantime! :)

Edit: And you've done it again :D

[lukeed]

We care about this lib & like working on it 😜

[lukeed]

Sorry, forgot to update this thread.

I played with auto-loading if plugins: true, but found that it's not 100% consistent. The problem is that load (and some of its internals) are coroutines, and you can't use 'em inside a class constructor.

The closest I got was to set up an .on('autoloaded') listener, but that's where the inconsistency pops up. It's asynchronous, so (older) machines may spend more time finding plugins & requiring them than expected. While that's happening, Tasks could have been started & already thrown due to the "methods not found" error.

I'll get back to this later, but it'll have to be a class method on Fly.

const fly = new Fly({ file: 'path/to/foobar.js' });

fly.autoload().then(() => {
  fly.start('taskA');
})

[adamkiss]

With big enough warning, it superawesome either way.

(the reason for the warning is that after all the magic fly brought, I spent literally few hours being confused as hell why it doesn't work—unloaded plugins was like 45th thing to check)

[karimsa]

I'm currently handling this via an external npm module I wrote - fly-load. You simply call the provided function and it returns a fly instance with the modules loaded.

The code could definitely be added to the fly project natively - I can look into opening a PR for this?