lukeed / taskr

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

Add inline `plugin` method #233

Closed devmondo closed 7 years ago

devmondo commented 7 years ago

hi, sometimes i have project specific functionality and i was wondering how can i create a plugin in the same flyfile instead of going through the whole process of creating npm package.

thanks in advanced.

lukeed commented 7 years ago

Hey,

Do you have a preference on how this would be done? The only way it's doable now (without NPM) is creating an external plugin & then linking it thru your package.json

So something like:

// build/custom.js

module.exports = function () {
  this.plugin('methodName', {every: 0}, function * (files, opts) {
    // custom plugin function
    // PS: this way it's reusable!
  });
};
// package.json

{
  "devDependencies": {
    "fly-custom-whatever": "file:build/custom"
  }
}

It has to be listed with fly- anywhere in the dependencies for it to be registered.

lukeed commented 7 years ago

I know you're referring to the old .filter() method that could be used to do something "quick" from within your task.

The problem I see, now, with that is that it doesn't really align with the new-&-improved plugin() API anymore, and I'm not sure, personally, if I want to encourage non-reusable methods. 🤔

devmondo commented 7 years ago

@lukeed man you never sleep or rest! :) yes you were right there, i refer to the filter old method, and i agree with you i just thought if it is possible as quick solution for simple stuff.

lukeed commented 7 years ago

@devmondo lol, there's too much I want to do!

You can be honest; do you think my suggestion isn't quick enough?

ping @jbucaran @hzlmn

devmondo commented 7 years ago

@lukeed before answering i tried to do what you said, i added fly-test.js to the root of my project

module.exports = function () {
    this.plugin('test', {every: 0}, function * (files, opts) {
        console.log(files,opts);
    });
};

and added this to my project package.json devDependencies section "fly-test":"file:fly-test"

in flyfile

exports.testMe = function *(o) {
    yield this.source(o.src || paths.merge)
        .test()

};

i get error TypeError: this.source(...).test is not a function

i know i am doing something wrong!

lukeed commented 7 years ago

@devmondo If you're using the file: spec, NPM needs it to be relative pathing. So I'm pretty sure file:./fly-test will work.

devmondo commented 7 years ago

done that still same error, hope i am not wasting your time with something else i am missing

hzlmn commented 7 years ago

@devmondo I think it is because fly can not find module inside node_modules. You need to add dependency and then run npm install, it will place folder in your local modules.

I had same task. I just made a hack directly binding a plugin func to fly instance inside task.

lukeed commented 7 years ago

@devmondo Oh, hm... I had it a bit different within the tests.

But yea, what I wrote was incomplete. I got it working by adding another package.json for the new task... Not very ideal.

// build/fly-custom/index.js
module.exports = function () {
    this.plugin('custom', {every: 0}, function * (files, opts) {
        console.log(files,opts);
    });
};

// build/fly-custom/package.json
{
  "name": "fly-custom",
  "version": "0.0.0"
}

// package.json
{
  "devDependencies": {
    "fly-custom": "file:build/fly-custom"
  }
}

And because this was added after everything else was installed, you have to run:

npm link ./build/fly-custom

This will copy the fly-custom package to your node_modules.

You won't have to do this the next time you npm install because it's done automatically.


If you have multiple custom plugins, another approach would be to do this:

// build/fly-custom-plugins/index.js
module.exports = function () {
    this.plugin('plugOne', {every: 0}, function * (files, opts) {
        // ...
    });

    this.plugin('plugTwo', {}, function * (files, opts) {
        // ...
    });

    this.plugin('plugThree', {every: 0}, function * (files, opts) {
        // ...
    });
};

// build/fly-custom-plugins/package.json
{
  "name": "fly-custom-plugins",
  "version": "0.0.0"
}

// package.json
{
  "devDependencies": {
    "fly-custom-plugins": "file:build/fly-custom-plugins"
  }
}
devmondo commented 7 years ago

@hzlmn you are right! it is working now thank you very much but boy it is some work ahhaha

@lukeed thank you i came to the same conclusion that i had to create package.json.

if having inline plugin is gonna break your philosophy well i think we can live with this method, but if it is not then it would be great as current methodt requires some work and confusing to people i pass project to, because they would not understand first minute that plugin package is in same project hahahah but again we can live with that :)

thanks again all

lukeed commented 7 years ago

It became apparent that it's needed, haha.

Any suggestions on a method name? No on filter

devmondo commented 7 years ago

some shameful suggestions inlinePlugin feature extension extend

lukeed commented 7 years ago

I'm thinking: pipe (a la gulp), run, inject.

yield this.source(...).run({every: 0}, function * (files) {
    console.log('these are all matched files: ', files)
  }).target('dist')

// use default settings; aka loop every file
yield this.source(...).run({}, function * (file) {
    console.log('this is a single source file object: ', file)
  }).target('dist')
devmondo commented 7 years ago

@lukeed pipe sound great.

on a side note , if i may ask when will v2 docs will be ready? i see cool features in your samples above run, inject i did not know of!

lukeed commented 7 years ago

@hzlmn what say you?

@devmondo they're coming up soon. It makes most sense to me for them to be last since things may still change

hzlmn commented 7 years ago

IMO, don't like pipe as it intercepts with streams api and can be confusing for end user.

devmondo commented 7 years ago

@hzlmn good point, as silly as it sound, the positive side effect is that people who come from or want to replace Gulp will find them selves understanding Fly

lukeed commented 7 years ago

I've decided on .run. Active verb, implies immediate execution.

devmondo commented 7 years ago

@lukeed works for me :)

lukeed commented 7 years ago

Added in the new method (haven't pushed yet)!

Thinking about adding another option, but looking for input first....

Because adding local plugins (as recommended above for "reusable" private plugins) was so cumbersome, I think this would be very useful:

// package.json
{
  "name": "my-project",
  "private": "true",
  "devDependencies": {
    "fly": "beta",
    "fly-babel": "*",
    // ...
  },
  "fly": {
    "localPlugins": [
      "./build/custom-task-one.js",
      "./build/custom-task-two.js",
      "./build/custom-task-three.js"
    ]
  }
}

Of course, you can still require public modules, or require local modules using file:/ and npm link, but this will allow for quick, local require()s.

My only hesitation is the name of this key: localPlugins. I don't want to call it plugins because users may mistake it as the place to declare all plugins (including public). Other options are import, require ... etc, but since the key is only for plugins, maybe the name should reflect that?

lukeed commented 7 years ago

Revisiting this; would appreciate your feedbacks :)

What if the .run method were renamed of .plugin or .inline?

For plugin():

For inline():

The reason I'm thinking about switching is that .run() can be confused for running a new task?

Here's what they look like:

// current
exports.default = function * (fly) {
  yield fly.source('src/*.js').run({every: true}, function * (file) {
    console.log(`print every file; eg: ${file.base}`);
  }).target('dist');
}

// plugin
exports.default = function * (fly) {
  yield fly.source('src/*.js').plugin({every: true}, function * (file) {
    console.log(`print every file; eg: ${file.base}`);
  }).target('dist');
}

// inline
exports.default = function * (fly) {
  yield fly.source('src/*.js').inline({every: true}, function * (file) {
    console.log(`print every file; eg: ${file.base}`);
  }).target('dist');
}
lukeed commented 7 years ago

Also, I'm likely going to make the function parameter optional & accept a func key on the object so that shorthand notation can be used.

Here's what that will look like; in case it changes your minds for the name of the method. 😆

// current
exports.default = function * (fly) {
  yield fly.source('src/*.js').run({
    every: true,
    *func(file) {
      console.log(`print every file; eg: ${file.base}`);
    }
  }).target('dist');
}

// plugin
exports.default = function * (fly) {
  yield fly.source('src/*.js').plugin({
    every: true,
    *func(file) {
      console.log(`print every file; eg: ${file.base}`);
    }
  }).target('dist');
}

// inline
exports.default = function * (fly) {
  yield fly.source('src/*.js').inline({
    every: true,
    *func(file) {
      console.log(`print every file; eg: ${file.base}`);
    }
  }).target('dist');
}
lukeed commented 7 years ago

@devmondo @hzlmn Your feedback would be greatly appreciated! 😸