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

Generator._queues doesn't refresh after Env.create, Env.instantiate or Generator.composeWith #241

Closed
manuth opened this issue Jul 30, 2020 · 8 comments

Comments

@manuth
Copy link

manuth commented Jul 30, 2020

In order to extend generators with customPriorities correctly, I'd expect that the _queues of every generator in an Environment are refreshed as soon as a new priority is added to the Environment.runLoop.

Reproducing the Error

Create a generator with a step cleanup which should execute before end by passing these options to super:
./lib/generators/a/index.js

const Generator = require("yeoman-generator");
class GeneratorA extends Generator {
    constructor(args, options) {
        super(
            args,
            {
                ...options,
                customPriorities: [
                    {
                        priorityName: "cleanup",
                        before: "end"
                    }
                ]
            });
    }

    public install() { console.log("a:install"); }
    public cleanup() { console.log("a:cleanup"); }
    public end() { console.log("a:end"); }
}

Create a generator which inherits said generator:
./lib/generators/b/index.js

const { join } = require("path");
const Generator = require("yeoman-generator");
class GeneratorB extends Generator {
    constructor(args, options) {
        super(args, options);
        this.composeWith(join(__dirname, "..", "a"); // or `this.env.create` or `this.env.instantiate`
    }

    public install() { console.log("b:install"); }
    public cleanup() { console.log("b:cleanup"); }
    public end() { console.log("b:end"); }
}

Expected Result

After manipulating the runLoop of the environment, the queues of the generator should be reordered.
Therefore the console output should look as followed:
In case you invoked this.composeWith:

b:install
a:install
b:cleanup
a:cleanup
b:end
a:end

In case you invoked this.env.create or this.env.instantiate:

b:install
b:cleanup
b:end

Actual Result

Instead of said console output, following is logged:
In case you invoked this.composeWith:

b:cleanup
b:install
a:install
a:cleanup
b:end
a:end

In case you invoked this.env.create or this.env.instantiate:

b:cleanup
b:install
b:end
@mshima
Copy link
Member

mshima commented Jul 30, 2020

Try to debug with env DEBUG=yeoman:generator.
But looks like b:cleanup is been queued at default priority.

@manuth
Copy link
Author

manuth commented Jul 30, 2020

Yeah, exactly.
It's because the _queues array is only built once in the Generator constructor here:
https://github.com/yeoman/generator/blob/245fcf53c8ff0baa00dbc86fd5be3fb47995bce1/lib/index.js#L292-L298
And here:
https://github.com/yeoman/generator/blob/245fcf53c8ff0baa00dbc86fd5be3fb47995bce1/lib/index.js#L332
But an up to date _queues is required to enqueue tasks correctly as seen here:
https://github.com/yeoman/generator/blob/245fcf53c8ff0baa00dbc86fd5be3fb47995bce1/lib/index.js#L863

With the _queues array not being updated on Environment.create, Environment.instantiate or Generator.composeWith, tasks will be added to the default queue instead.

So not even invoking Generator.startOver would solve this issue, as startOver doesn't re-invoke the constructor but rather Generator.queueOwnTasks.

@mshima
Copy link
Member

mshima commented Jul 30, 2020

@manuth I really don't get what you are trying to explain.
Generator.composeWith calls Environment.create, which calls Environment.instantiate, which calls the generator constructor, so Generator._queues will be created and populated once.
Generator._queues is not changed anywhere else so it's updated.

Without a real test case, I think the problem is A or B generator constructor not passing customPriorities option correctly.

@mshima
Copy link
Member

mshima commented Jul 30, 2020

Are you creating another custom priority at B? If so you need to change A constructor.

const Generator = require("yeoman-generator");
class GeneratorA extends Generator {
    constructor(args, options) {
        super(
            args,
            {
                ...options,
                customPriorities: [
                    ...options.customPriorities,
                    {
                        priorityName: "cleanup",
                        before: "end"
                    }
                ]
            });
    }

    public install() { console.log("a:install"); }
    public cleanup() { console.log("a:cleanup"); }
    public end() { console.log("a:end"); }
}

@manuth
Copy link
Author

manuth commented Jul 30, 2020

@manuth I really don't get what you are trying to explain.
Generator.composeWith calls Environment.create, which calls Environment.instantiate, which calls the generator constructor, so Generator._queues will be created and populated once.
Generator._queues is not changed anywhere else so it's updated.

Without a real test case, I think the problem is A or B generator constructor not passing customPriorities option correctly.

This is exactly what's causing the problem.
If you execute Generator.composeWith in GeneratorA you would expect, that GeneratorA inherits the customPriorities from GeneratorB, wouldn't you?
But this is not the case, because Generator._queues is only populated once inside the constructor.

@mshima
Copy link
Member

mshima commented Jul 30, 2020

@manuth I really don't get what you are trying to explain.
Generator.composeWith calls Environment.create, which calls Environment.instantiate, which calls the generator constructor, so Generator._queues will be created and populated once.
Generator._queues is not changed anywhere else so it's updated.
Without a real test case, I think the problem is A or B generator constructor not passing customPriorities option correctly.

This is exactly what's causing the problem.
If you execute Generator.composeWith in GeneratorA you would expect, that GeneratorA inherits the customPriorities from GeneratorB, wouldn't you?
But this is not the case, because Generator._queues is only populated once inside the constructor.

No, custom priorities are individual queues, there is no way to share a custom priority.
Actually I was thinking GeneratorB extended GeneratorA, so GeneratorA options would create the custom priority for GeneratorB.

  • What if you compose with GeneratorB without composing with GeneratorA?
  • What if you compose GeneratorA with GeneratorC that implements a cleanup task that must be executed at default priority?

Both cases will cause unexpected behaviours.
So custom priorities are individual, you can share a grouped queue, but I recommend to don't put generic names:

               customPriorities: [
                    {
                        priorityName: "cleanup",
                        before: "end",
                        queueName: "myPackage:cleanup"
                    }
                ]

@mshima
Copy link
Member

mshima commented Jul 30, 2020

I recommend to create something like:

const Generator = require("yeoman-generator");
class GeneratorWithCleanup extends Generator {
    constructor(args, options) {
        super(
            args,
            {
                ...options,
                customPriorities: [
                    ...options.customPriorities,
                    {
                        priorityName: "cleanup",
                        queueName: "myPackage:cleanup",
                        before: "end"
                    }
                ]
            });
    }
}

And make GeneratorA and GeneratorB extend GeneratorWithCleanup

@manuth
Copy link
Author

manuth commented Jul 30, 2020

Got it!
Thanks for your recommendation

@manuth manuth closed this as completed Jul 30, 2020
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

2 participants