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

Custom Tasks Workflow #4

Closed
zewa666 opened this issue Oct 27, 2016 · 12 comments
Closed

Custom Tasks Workflow #4

zewa666 opened this issue Oct 27, 2016 · 12 comments
Assignees

Comments

@zewa666
Copy link
Contributor

zewa666 commented Oct 27, 2016

So the ideal workflow for Aurelia-I18N together with Pacman would be to not only automatically wire up the aurelia.json file with its dependencies, but also initially allow the user to choose whether he wants his main.js file setup with the plugin initialization + other stuff.

I've started playing with that by adding a custom i18n-setup.js task into the task folder and wiring up the script section inside the aurelia-i18n registry entry. This is a pretty cool idea btw, but here are some additional thoughts.

Auto-detect tasks

It would be nice if during installation, Pacman could inspect the package, look for a specific folder, e.g. tasks, and use all files from in there, which are in a specific format --> e.g. task-[type].js => task-install.js

Dependencies

If we go the above mentioned path, the benefit would also be that the package itself can maintain all necessary dev-dependencies. Like in my case I'd need a gulp-replace dependency and currently I'd have to install it as part of Pacman, which would get quickly out of hand if everybody does that.
Any other ideas how to handle this?

Prompted tasks

Maybe not every task should be executed immediately, so it would be nice to have a way to let the user confirm before execution. So thinking of that maybe the default could be that a generic prompt is always fired and a the user can silently agree to all of them by passing in an additional parameter. E.g.

au pacman i aurelia-i18n --q --> q for quiet

Scaffolding templates

During initialization of i18n, one part of the story would be to create a default translation file like locales/en/translation.json. It would be helpful to prepare said file as a template and just copy it to the target destination during the task run. How and where should those templates be placed. Again I think if we exclude the taskhandling from Pacman and let the package maintain it by itself, we wouldn't have to take care of it, as mentioned in the first point.

Custom message

The install process nicely describes what is happening, but it would be great if at the end, each task could output a custom success/error message in order to let the user know what exactly happened.


Ok so a pretty long list, I don't mind if we split it up on several github issues if necessary, just wanted to check back with you first.

@martonsagi
Copy link
Owner

Good ideas! It would be awesome if you could join to the conversation about Monterey integration here. Above proposals certainly have a place in that discussion.
(And I have to admit that I just have no idea how to reference this issue there :))

@martonsagi martonsagi self-assigned this Oct 27, 2016
@martonsagi
Copy link
Owner

martonsagi commented Nov 2, 2016

@zewa666
I've reorganized the codebase with future aurelia cli-integration and above listed features in mind.
New structure allows injection of custom workflows at package level.

Analyzer

Gathers basic information, such as aurelia.json, command line parameters, custom hooks placed within the installed package.

ImportEngine

Configures and executes all available custom importer steps according to information defined by <metadata.json>.

Example flow:

  • Steps: ['applyPatches', 'registerDependencies']
  • Importers: [ImportBase, MyCustomImport, SecondCustomImport]
  1. Step: applyPatches
    • Order:
      1. ImportBase.applyPatches()
      2. MyCustomImport <-- applyPatches() cannot be found, skipping
      3. SecondCustomImport.applyPatches()
  2. Step: registerDependencies
    • Order:
      1. ImportBase.applyPatches()
      2. MyCustomImport.registerDependencies()
      3. SecondCustomImport <-- registerDependencies() cannot be found, skipping

ImportBase (Custom workflows)

Example implementation for metadata file processing. This runs first by default, and you can add any number of your own implementations as well.
I've created an autodiscovery feature, which is capable of detecting hooks placed into the package itself (<node_modules>/<package>/install/import-hooks.js).

Built-in steps in execution order (it follows the format of metadata file.)

(0. register)

  1. applyPatches
  2. registerDependencies
  3. registerBundles
  4. saveProject
  5. installTasks
  6. executeScripts

Example for custom hooks:

module.exports = class {

    constructor() { }

    register(engine, options) {
        this.engine = engine;
        this.cliParams = options;

        this.engine.availableSteps.pop('runBeforePatches');
    }

    // newly added custom step
    runBeforePatches() {
        // my customized operations

        if (this.cliParams.quiet) {
            console.log('I can display my own messages too ;)');
        }
    }

    registerBundles() {
        // additional todos after ImportBase.registerBundles() finished
    }

Providers (Package Management)

There is currently one integration with NPM. As I'm not sure how much it's going to be needed, it's been commented out by default in pacman.js task. I kept the possibility open anyway.

Installed au pacman task

It uses a default implementation of above classes, but its parts can be replaced or overridden if desired.

Example modifications:

let analyzer = new Analyzer(CLIOptions);

    analyzer.prototype.autoDiscoverHooks = (pkgName) => {
        // my custom implementation
    };

    analyzer
        // analyze contextual information (CLI parameters, given package)
        .execute()

        // manage packages using MyFavouritePackageManager
        .then(result => {
            let myProvider = new MyFavouritePackageManagerProvider();
            return myProvider[result.options.action](result.options.pkg);
        })

        // configure aurelia.json, install custom tasks, run additional scripts
        .then(() => {
            let engine = new ImportEngine(
                analyzer.result.project,
                analyzer.result.config,
                [new MyCustomImportBase(), new SecondCustomImport(), analyzer.result.importer]
            );

            return engine.execute(analyzer.result.options);
        })
        .catch(e => console.log(e));

After writing that example down, I've just realized a semantic error. When using NPM integrated mode, analyzer runs before NPM installation. Therefore package's custom hooks cannot be detected properly.
I'll come up with something to solve this.


Above changes can provide support for several of your ideas:

  • Auto-detect tasks
  • Scaffolding templates
  • Custom message

Dependencies
If we go the above mentioned path, the benefit would also be that the package itself can maintain all necessary > dev-dependencies. Like in my case I'd need a gulp-replace dependency and currently I'd have to install it as part of Pacman, which would get quickly out of hand if everybody does that.
Any other ideas how to handle this?

You can use the scripts section in the metadata file to add new packages. It's also possible to place custom CLI tasks into the package as well. (Related commit for task autodiscovery hasn't been pushed yet, as I did forget it last night. :). I'll send it in later today).

Example aurelia-i18n.json:

{
    "patches": null,
    "dependencies": [
        ...
    ],
    "tasks": [
      "i18nscaffold"
    ]
    "scripts": {
      "install": [
        "npm i gulp-replace"
        "au i18nscaffold" <-- run task which requires gulp-replace
      ]
    }
}

Prompted tasks
Maybe not every task should be executed immediately, so it would be nice to have a way to let the user confirm > before execution. So thinking of that maybe the default could be that a generic prompt is always fired and a the user can silently agree to all of them by passing in an additional parameter. E.g.

I'll dig myself into aurelia-cli's source to provide such possibility using existing aurelia-cli infrastructure.
There is a new --quiet|q option, but it only hides the console.log() output at this moment.

@zewa666
Copy link
Contributor Author

zewa666 commented Nov 2, 2016

perfect, sounds like there is enough in place to continue experimenting with i18n. I'll come back to you with more insights

@martonsagi
Copy link
Owner

Thanks. I've just sent out updates to github and npm. You can update to v.0.1.0 with npm as well.

@zewa666
Copy link
Contributor Author

zewa666 commented Nov 5, 2016

Ok so I gave the thing a bit of a look. So far looks good, yet I have some workflow understanding questions.

So before your change I did expand registry/aurelia-i18n.jswith

    "scripts": {
        "install": [
            "au i18n-setup"
        ]
    }

and placed a file i18n-setup.js into the tasks folder which essentially is a gulp task doing stuff I need in order to setup the i18n. As seen above the install section will run this task then.

So with your new changes we can actually skip this part right? I'd leave the registry entry without the script section. Then all I'd need to do is create a file install/import-hooks.js inside the aurelia-i18n repo. Since aurelia-i18n can declare the necessary devDeps, also for the setup part, by itself, I don't need any special API for that.
All I'd do is to create something like this:

class I18NImportHooks {
    constructor() {}

    executeScripts() {
        // setup the main.js of the project here
    }
}

module.exports = I18NImportHooks;

What I'm not sure now is the following. I'd need a path reference relative to the users scaffolded app. So ideally the constructor could get some metadata injected. The injected meta could provide several info, one of them e.g the absolute path to the scaffolded projects root projectRoot. From there on I wouldn't need any gulp or whatever but could do that solely with pure VanillaJS itself.
Of course I could also find the root by ../../../ but this somehow feels a bit sub-optimal.

class I18NImportHooks {
    constructor(meta) {
      this.meta = meta;
    }

    executeScripts() {
        // setup the main.js of the project here
        var projectMainFile = this.meta.projectRoot + "/src/main.js";
    }
}

So overall did I get your idea right? If so that's great, because this way the registry file doesn't need to do anything besides stating the deps and helping pacman to find the plugin. Everything else can be completely done by the plugin itself without any Gulp tasks or whatever.

@martonsagi
Copy link
Owner

So with your new changes we can actually skip this part right?

That is correct. If you'd like to do that, you have the freedom to place all of your custom logic within import-hooks.executeScripts method. Actually, you could use any of the workflow steps to execute scripts and do custom stuff or even change their order of execution.

The scripts.install section might be useful in some cases. Let's say you have a i18n-config custom cli task with several capabilities, which is needed to be installed for later use, and to be executed by pacman as well.

Example task: configures import, edits translation.json files

au i18n-config [--import] [--add <lang,key,translation>]

Import setup:

"tasks": [
    "i18n-config"
],
"scripts": {
    "install": [
        "au i18n-config --import"
    ]
}

After importing package, we might want to use au i18n-config --add hu,Name,Név or similar from time to time.
My intention here was to provide developers with several options, whichever suits them better.


What I'm not sure now is the following. I'd need a path reference relative to the users scaffolded app. So ideally the constructor could get some metadata injected. The injected meta could provide several info, one of them e.g the absolute path to the scaffolded projects root projectRoot. From there on I wouldn't need any gulp or whatever but could do that solely with pure VanillaJS itself.
Of course I could also find the root by ../../../ but this somehow feels a bit sub-optimal.

I've updated ImportEngine to pass a projectInfo object to import hooks. I haven't found any better solution yet, other than using ../../../.

We can extend this later to hold more information, if needed.

registerImporters(options) {
    let projectInfo = {
        projectRoot: path.join(__dirname, '..', '..', '..')
    };
    ...
}

The import-hooks.js can have an optional register method, which will be called by ImportEngine. I'd recommend using this for general initialization.

It would normally look similar to this:

    register(engine, options, projectInfo) {
        this.engine = engine;
        this.cliParams = options;
        this.projectInfo = projectInfo;

        // other custom stuff
    }

Applying above to the import-hooks.js file:

class I18NImportHooks {
    constructor() { }

    register(engine, options, projectInfo) {
        this.engine = engine;
        this.cliParams = options;
        this.projectInfo = projectInfo;
    }

    executeScripts() {
        // setup the main.js of the project here
        var projectMainFile = this.projectInfo.projectRoot + "/src/main.js";

        // ** magic **
    }
}

module.exports = I18NImportHooks;

I don't use constructor on purpose. You can pass any number of import hook instances to ImportEngine, therefore object initialization might be out of pacman's control. Not all hooks will be created by autodiscovery function, that's why I've created this common register method to be there.


So overall did I get your idea right?

Absolutely! :)

@zewa666
Copy link
Contributor Author

zewa666 commented Nov 6, 2016

Perfect, great ideas and I'm currently working out my setup script. Prompting the user would be a really nice thing but the initial draft could already work without it. As soon as the setup thingy is done I'm gonna publish that to the plugins repo, as I do see so much value just by that alone.

But yeah you had great ideas for tasks like adding new translations or import from existing files.

@martonsagi
Copy link
Owner

Fantastic, I'm looking forward to see your setup in action!

Prompting the user would be a really nice thing

I've looked into that how it is handled by aurelia-cli itself. Well, the good news is that I was able to use their built-in UI class in my experimental setup.
However, in order to successfully integrate this into pacman, I'll need to redesign ImportEngine and import-hook steps, since ui.question() is async.

I'll come up with some (hopefully) backward-compatible solution next week.

@martonsagi
Copy link
Owner

I'll come up with some (hopefully) backward-compatible solution next week.

I did it without breaking changes. Import steps still run in precise order, but they can return promises optionally. This change allows us to be able to pause the import process until a question is answered.

There's a new CliBridge class, which is a placeholder for all aurelia-cli features needed in pacman. At this moment, ConsoleUI is the only feature available. ImportEngine has an instance of it, so import-hooks can have access to it.

Putting it together, import-hooks could look like this:

class I18NImportHooks {
    constructor() { }

    register(engine, options, projectInfo) {
        this.engine = engine;
        this.cliParams = options;
        this.projectInfo = projectInfo;
        this.ui = this.engine.cliBridge.ui; // new line
    }

    executeScripts() {
        let options = [
            {
              "displayName": "Proceed",
              "description": "Proceed description",
              "value": "ok"
            },
            {
              "displayName": "Cancel",
              "description": "Cancel description",
              "value": "cancel"
            },
            {
              "displayName": "Restart",
              "description": "Restart description",
              "value": "restart"
            }
        ];

        return this.ui.question('Choose an option...', options).then(answer => {
            // setup the main.js of the project here
            let projectMainFile = this.projectInfo.projectRoot + "/src/main.js";

            // ** magic **
        });
    }
}

module.exports = I18NImportHooks;

I don't use it for built-in steps yet. There are 5 steps, so turning 5 questions on by default would be very annoying in my opinion. :D It might be a better fit for optional input requests (e.g. filename to save to), or choosing from multiple options.

Maybe instead of changing --quiet flag to disable confirms, we could add a --verbose flag, which would require confirmation before every step.

@zewa666
Copy link
Contributor Author

zewa666 commented Nov 8, 2016

thats great. So for me this issue can be closed since all things got addressed. Awesome work @martonsagi

@martonsagi
Copy link
Owner

Thank you!

@adriatic
Copy link

adriatic commented Nov 8, 2016

There is so much overlap between what is done here and what will be done in aurelia-cli package importer that I would suggest to @zewa666 to continue watching the evolution of the aurelia-cli package importer. Since @EisenbergEffect is already participating, we should see a really cool addition to Aurelia.

cc: @JeroenVinke

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

No branches or pull requests

3 participants