Undefined variables retrivied by .yo-rc.json in the blueprint app generator customization

ghost commented 4 years ago

Overview of the issue

From the jhipster 6.6.0 version due to the #10752 PR , it's possible to extend the app generator in the blueprint. It is a great thing, but the problem is that the variables saved by the yeoman config.set method, add all under the blueprint key name in the .yo-rc.json file. So in the re-generation (or the import-jdl command with the config specified in the jdl model), the variables retrieved in the .yo-rc.json file by the this.config.get(variableName) method of the jhipster standard app index.js, are undefined because they are not saved under the "generator-jhipster" key of the yo-rc.json file.

Motivation for or Use Case

The motivation is that the yeoman-generator standard class uses a storage class to save configuration in the .yo-rc.json file. For this, it uses a method to define the key name using the package.json name field of the project class that extends that class. So these variables saved by the jhipster app generator, are stored in the blueprint name key instead of "generator-jhipster" key name.

Reproduce the error

In my scenario, the blueprint name is generator-jhipster-nodejs. Run the following command without a yo-rc.json file in the root folder: jhipster --blueprints nodejs

After answering all the questions, I have:

.yo-rc.json file

{
  "generator-jhipster-nodejs": {
    "jhipsterVersion": "6.6.0",
    "applicationType": "monolith",
    "baseName": "jhipster",
    "packageName": "com.jhipster.node",
    "packageFolder": "com/jhipster/node",
    "serverPort": "8081",
    "authenticationType": "jwt",
    "cacheProvider": "no",
    "enableHibernateCache": false,
    "websocket": false,
    "databaseType": "sql",
    "devDatabaseType": "sqlite",
    "prodDatabaseType": "sqlite",
    "searchEngine": false,
    "messageBroker": false,
    "serviceDiscoveryType": false,
    "buildTool": "maven",
    "enableSwaggerCodegen": false,
    "embeddableLaunchScript": false,
    "useSass": true,
    "clientPackageManager": "npm",
    "clientFramework": "angularX",
    "clientTheme": "none",
    "clientThemeVariant": "",
    "creationTimestamp": 1577471930710,
    "testFrameworks": [],
    "jhiPrefix": "jhi",
    "entitySuffix": "",
    "dtoSuffix": "DTO",
    "otherModules": [
      {
        "name": "generator-jhipster-nodejs",
        "version": "1.0.0-beta.2"
      }
    ],
    "enableTranslation": false,
    "blueprints": [
      {
        "name": "generator-jhipster-nodejs",
        "version": "1.0.0-beta.2"
      }
    ]
  }
}

I rerun the previous command, and the first error is the 'blueprints' properties undefined:

events.js:174
      throw er; // Unhandled 'error' event
      ^
TypeError: Cannot read property 'blueprints' of undefined
    at loadBlueprintsFromConfiguration (C:\Users\Sweet\Desktop\github\generator-
jhipster-nodejs\node_modules\generator-jhipster\generators\utils.js:565:38)
    at Object.getAllJhipsterConfig (C:\Users\Sweet\Desktop\github\generator-jhip
ster-nodejs\node_modules\generator-jhipster\generators\utils.js:504:36)
    at module.exports.getAllJhipsterConfig (C:\Users\Sweet\Desktop\github\genera
tor-jhipster-nodejs\node_modules\generator-jhipster\generators\generator-base.js
:2060:30)
    at module.exports.setupServerconsts (C:\Users\Sweet\Desktop\github\generator
-jhipster-nodejs\node_modules\generator-jhipster\generators\server\index.js:134:
44)
    at Object.<anonymous> (C:\Users\Sweet\Desktop\github\generator-jhipster-node
js\node_modules\generator-jhipster\node_modules\yeoman-generator\lib\index.js:42
4:27)

Suggest a Fix

In the nodejs blueprint I overwrote the yeoman rootGeneratorName method in the custom app generator. In my opinion this method must be added in the jhipster main app together the rootGeneratorVersion method, using the jhipster package name and version:

 /**
     * Override yeoman standard storage function for yo-rc.json.
     * @return {String} The name of the root generator
     */
    rootGeneratorName() {
        return packagejs.name;
    }

    /**
     * Override yeoman standard storage function for yo-rc.json.
     * @return {String} The version of the root generator
     */
    rootGeneratorVersion() {
        return packagejs.version;
    }

In that way the .yo-rc.json stored will be:

.yo-rc.json file

{
  "generator-jhipster-nodejs": {
    "jhipsterVersion": "6.6.0",
    "applicationType": "monolith",
    "baseName": "jhipster",
    "packageName": "com.jhipster.node",
    "packageFolder": "com/jhipster/node",
    "serverPort": "8081",
    "authenticationType": "jwt",
    "cacheProvider": "no",
    "enableHibernateCache": false,
    "websocket": false,
    "databaseType": "sql",
    "devDatabaseType": "sqlite",
    "prodDatabaseType": "sqlite",
    "searchEngine": false,
    "messageBroker": false,
    "serviceDiscoveryType": false,
    "buildTool": "maven",
    "enableSwaggerCodegen": false,
    "embeddableLaunchScript": false,
    "useSass": true,
    "clientPackageManager": "npm",
    "clientFramework": "angularX",
    "clientTheme": "none",
    "clientThemeVariant": ""
  },
  "generator-jhipster": {
    "jhipsterVersion": "6.6.0",
    "creationTimestamp": 1577475384930,
    "applicationType": "monolith",
    "baseName": "jhipster",
    "testFrameworks": [],
    "jhiPrefix": "jhi",
    "entitySuffix": "",
    "dtoSuffix": "DTO",
    "otherModules": [
      {
        "name": "generator-jhipster-nodejs",
        "version": "1.0.0-beta.2"
      }
    ],
    "enableTranslation": false,
    "clientPackageManager": "npm",
    "blueprints": [
      {
        "name": "generator-jhipster-nodejs",
        "version": "1.0.0-beta.2"
      }
    ]
  }
}

And it doesn't raise any errors, in addition to be compatible with the standard jhipster configuration.

JHipster configuration

JHpster: 6.6.0

JHipster configuration, a `.yo-rc.json` file generated in the root folder

.yo-rc.json file

{
  "generator-jhipster-nodejs": {
    "jhipsterVersion": "6.6.0",
    "applicationType": "monolith",
    "baseName": "jhipster",
    "packageName": "com.jhipster.node",
    "packageFolder": "com/jhipster/node",
    "serverPort": "8081",
    "authenticationType": "jwt",
    "cacheProvider": "no",
    "enableHibernateCache": false,
    "websocket": false,
    "databaseType": "sql",
    "devDatabaseType": "sqlite",
    "prodDatabaseType": "sqlite",
    "searchEngine": false,
    "messageBroker": false,
    "serviceDiscoveryType": false,
    "buildTool": "maven",
    "enableSwaggerCodegen": false,
    "embeddableLaunchScript": false,
    "useSass": true,
    "clientPackageManager": "npm",
    "clientFramework": "angularX",
    "clientTheme": "none",
    "clientThemeVariant": "",
    "creationTimestamp": 1577471930710,
    "testFrameworks": [],
    "jhiPrefix": "jhi",
    "entitySuffix": "",
    "dtoSuffix": "DTO",
    "otherModules": [
      {
        "name": "generator-jhipster-nodejs",
        "version": "1.0.0-beta.2"
      }
    ],
    "enableTranslation": false,
    "blueprints": [
      {
        "name": "generator-jhipster-nodejs",
        "version": "1.0.0-beta.2"
      }
    ]
  }
}

**JDL for the Entity configuration(s) `entityName.json` files generated in

the .jhipster directory**

JDL entity definitions

Environment and Tools

java version "1.8.0_65" Java(TM) SE Runtime Environment (build 1.8.0_65-b17) Java HotSpot(TM) 64-Bit Server VM (build 25.65-b01, mixed mode)

git version 2.9.3.windows.2

node: v10.15.3

npm: 5.3.0

yeoman: 3.1.0

yarn: 1.3.2

Browsers and Operating System

OS: Windows and linux

[x] Checking this box is mandatory (this is just to show you read everything)

mshima commented 4 years ago

Seems duplicate of https://github.com/jhipster/generator-jhipster/issues/10663

ghost commented 4 years ago

@mshima thanks, I read that. But is it more simple to overwrite the yeoman generator name function in the app index jhipster generator? In this way it is compatibility with the standard jhipster code in case of no main app customization.

mshima commented 4 years ago

Changing rootGeneratorName is exactly the same of this.config = this._getStorage('generator-jhipster');

https://github.com/jhipster/generator-jhipster/blob/a2e9dfe53e670141b985e49c5ccec82fb99aeb5a/generators/generator-base-blueprint.js#L32

https://github.com/jhipster/generator-jhipster/pull/10664 Is about migrating old projects.

mshima commented 4 years ago

See https://github.com/yeoman/generator/pull/1122/files

This was merged at yeoman-generator 4.1.0. Jhipster depends on 3.2.0. So, it will not work with 6.6.0 blueprints.

mshima commented 4 years ago

rootGeneratorVersion does nothing here. It’s used at global config only.

https://github.com/yeoman/generator/search?utf8=✓&q=rootGeneratorVersion&type=

ghost commented 4 years ago

rootGeneratorVersion does nothing here. It’s used at global config only.

https://github.com/yeoman/generator/search?utf8=✓&q=rootGeneratorVersion&type=

Yes in jhipster is not used, I added that only because it is associated with the generator name, so if I change the name function sequentially is usefull to specify the related version

ghost commented 4 years ago

See https://github.com/yeoman/generator/pull/1122/files

This was merged at yeoman-generator 4.1.0. Jhipster depends on 3.2.0. So, it will not work with 6.6.0 blueprints.

It's not mandatory to update yeoman-generator for 4.x version. The rootGeneratorName method is already added in the 3.2.0 version : https://github.com/yeoman/generator/blob/3bbeef0f8eebd02e9459734c9a3d9c6732a47d14/lib/index.js#L597 . From yeoman-generator 4.1.0 is been only modified the getStorage() method with the generator name argument. So it works for jhipster 6.6.0, because my suggestion is only to overwrite that method, used in the storage class, in the jhipster app index generator (and not use the getStorage method passing the generator name as argument directly in the jhipster app).

mshima commented 4 years ago

Please don’t change rootGeneratorName(). It’s a bad workaround and will break my blueprint.

This can be done at the blueprint, doesn’t need to be at generator-jhipster.

ghost commented 4 years ago

Please don’t change rootGeneratorName(). It’s a bad workaround and will break my blueprint.

This can be done at the blueprint, doesn’t need to be at generator-jhipster.

Why will break your blueprint? It is an override only for the app index generator to use its standard config (and not of the blueprint). For me it's a function to add in the main app generator (and not in the blueprint), because it resolve a break of the jhipster extension not due to the blueprint itself but for jhipster configuration mode.

mshima commented 4 years ago

It is an override only for the app index generator to use its standard config (and not of the blueprint)

Currently there isn’t a standard config (generator-jhipster namespace) and a blueprint config (generator-jhipster-something namespace). There is only a standard config for not blueprinted generators and blueprint config for blueprinted generators.

For me it's a function to add in the main app generator (and not in the blueprint)

When you blueprint a generator, the main generator doesn’t run the normal cycle. It only instantiate the blueprinted version and stop.

https://github.com/jhipster/generator-jhipster/blob/5ae2e7a4aabbbde376102e36c77ba45ce901fd2d/generators/app/index.js#L344

The main generator and blueprinted generator doesn’t run together. So you are proposing to change a main generator function with the purpose to change the blueprint config object. This will unnecessary change rootGeneratorName behavior (my blueprint uses it) for every blueprint.

ghost commented 4 years ago

When you blueprint a generator, the main generator doesn’t run the normal cycle. It only instantiate the blueprinted version and stop.

It's not totally exact, because initially, creating a blueprint with the generator-jhipster-blueprint , I have classes for every subgenerator that inherit the standard jhipster phases. For example:

index.js of server subgenerator

/* eslint-disable consistent-return */
const chalk = require('chalk');
const ServerGenerator = require('generator-jhipster/generators/server');

module.exports = class extends ServerGenerator {
    constructor(args, opts) {
        super(args, { fromBlueprint: true, ...opts }); // fromBlueprint variable is important

        const jhContext = (this.jhipsterContext = this.options.jhipsterContext);

        if (!jhContext) {
            this.error(`This is a JHipster blueprint and should be used only like ${chalk.yellow('jhipster --blueprint test')}`);
        }

        this.configOptions = jhContext.configOptions || {};

        // This sets up options for this sub generator and is being reused from JHipster
        jhContext.setupServerOptions(this, jhContext);
    }

    get initializing() {
        /**
         * Any method beginning with _ can be reused from the superclass `ServerGenerator`
         *
         * There are multiple ways to customize a phase from JHipster.
         *
         * 1. Let JHipster handle a phase, blueprint doesnt override anything.
         * ```
         *      return super._initializing();
         * ```
         *
         * 2. Override the entire phase, this is when the blueprint takes control of a phase
         * ```
         *      return {
         *          myCustomInitPhaseStep() {
         *              // Do all your stuff here
         *          },
         *          myAnotherCustomInitPhaseStep(){
         *              // Do all your stuff here
         *          }
         *      };
         * ```
         *
         * 3. Partially override a phase, this is when the blueprint gets the phase from JHipster and customizes it.
         * ```
         *      const phaseFromJHipster = super._initializing();
         *      const myCustomPhaseSteps = {
         *          displayLogo() {
         *              // override the displayLogo method from the _initializing phase of JHipster
         *          },
         *          myCustomInitPhaseStep() {
         *              // Do all your stuff here
         *          },
         *      }
         *      return Object.assign(phaseFromJHipster, myCustomPhaseSteps);
         * ```
         */
        // Here we are not overriding this phase and hence its being handled by JHipster
        return super._initializing();
    }

    get prompting() {
        // Here we are not overriding this phase and hence its being handled by JHipster
        return super._prompting();
    }

    get configuring() {
        // Here we are not overriding this phase and hence its being handled by JHipster
        return super._configuring();
    }

    get default() {
        // Here we are not overriding this phase and hence its being handled by JHipster
        return super._default();
    }

    get writing() {
        // Here we are not overriding this phase and hence its being handled by JHipster
        return super._writing();
    }

    get install() {
        // Here we are not overriding this phase and hence its being handled by JHipster
        return super._install();
    }

    get end() {
        // Here we are not overriding this phase and hence its being handled by JHipster
        return super._end();
    }
};

So in my blueprint I can choose to leave the class in that way, and my generator executes the standard jhipster lifecycle for the server subgenerator. And all works well, also If I customize, for example only the initializing phase entirely or merging with the super._initializing():

const phaseFromJHipster = super._initializing();
return Object.assign(phaseFromJHipster, myCustomPhaseSteps);

But If I add an extension, without customize anything, in my blueprint for the app index in the same way:

index.js of app generator

/* eslint-disable consistent-return */
const chalk = require('chalk');
const AppGenerator = require('generator-jhipster/generators/app');

module.exports = class extends AppGenerator {
    constructor(args, opts) {
        super(args, { fromBlueprint: true, ...opts }); // fromBlueprint variable is important

        const jhContext = (this.jhipsterContext = this.options.jhipsterContext);

        if (!jhContext) {
            this.error(`This is a JHipster blueprint and should be used only like ${chalk.yellow('jhipster --blueprint test')}`);
        }

        this.configOptions = jhContext.configOptions || {};

        // This sets up options for this sub generator and is being reused from JHipster
        jhContext.setupServerOptions(this, jhContext);
    }

    get initializing() {
        /**
         * Any method beginning with _ can be reused from the superclass `AppGenerator`
         *
         * There are multiple ways to customize a phase from JHipster.
         *
         * 1. Let JHipster handle a phase, blueprint doesnt override anything.
         * ```
         *      return super._initializing();
         * ```
         *
         * 2. Override the entire phase, this is when the blueprint takes control of a phase
         * ```
         *      return {
         *          myCustomInitPhaseStep() {
         *              // Do all your stuff here
         *          },
         *          myAnotherCustomInitPhaseStep(){
         *              // Do all your stuff here
         *          }
         *      };
         * ```
         *
         * 3. Partially override a phase, this is when the blueprint gets the phase from JHipster and customizes it.
         * ```
         *      const phaseFromJHipster = super._initializing();
         *      const myCustomPhaseSteps = {
         *          displayLogo() {
         *              // override the displayLogo method from the _initializing phase of JHipster
         *          },
         *          myCustomInitPhaseStep() {
         *              // Do all your stuff here
         *          },
         *      }
         *      return Object.assign(phaseFromJHipster, myCustomPhaseSteps);
         * ```
         */
        // Here we are not overriding this phase and hence its being handled by JHipster
        return super._initializing();
    }

    get prompting() {
        // Here we are not overriding this phase and hence its being handled by JHipster
        return super._prompting();
    }

    get configuring() {
        // Here we are not overriding this phase and hence its being handled by JHipster
        return super._configuring();
    }

    get default() {
        // Here we are not overriding this phase and hence its being handled by JHipster
        return super._default();
    }

    get writing() {
        // Here we are not overriding this phase and hence its being handled by JHipster
        return super._writing();
    }

    get install() {
        // Here we are not overriding this phase and hence its being handled by JHipster
        return super._install();
    }

    get end() {
        // Here we are not overriding this phase and hence its being handled by JHipster
        return super._end();
    }
};

My blueprint generator already breaks. The reason is that the jhipster app generator does not manage well blueprint extension in the same way of server, client, entity subgenerators. Why? Because it set its variables with the yeaman this.config method with the namespace of the blueprint. It's the same thing done for every subgenerator, but for the app generator does not work because the standard jhipster phase app, inherited by the blueprint, want to read configuration in its namespace ("generator-jhipster") and therefore the variables retrivied are undefined: https://github.com/jhipster/generator-jhipster/blob/57ea50ad7d3654c5308690d6e8bd43e3932dae97/generators/app/index.js#L292

And raise after the error:

events.js:174
      throw er; // Unhandled 'error' event
      ^
TypeError: Cannot read property 'blueprints' of undefined
    at loadBlueprintsFromConfiguration (C:\Users\Sweet\Desktop\github\generator-
jhipster-nodejs\node_modules\generator-jhipster\generators\utils.js:565:38)

So to resolve this just add this method in the jhipster main index app generator:

/**
     * Override yeoman standard storage function for yo-rc.json.
     * @return {String} The name of the root generator
     */
    rootGeneratorName() {
        return packagejs.name;
    }

Where packagejs.name is "generator-jhipster" string. In this way the blueprint app class, that inherited this, does not break. Also if you don't add an app extension class in your blueprint, your blueprint does not break because in all cases the configuration (for the app generator) are retrivied well in the "generator-jhipster" key from the .yo-rc.json (and it works also if you run jhipster without blueprint).

So it is not a problem of the blueprint, but an error beheviour of the jhipster app generator when it is used as inherited class ( but for the other subgenerator all works well).

So you are proposing to change a main generator function with the purpose to change the blueprint config object.

No, because I'm not changing the blueprint config object and I'm not customizing anything yet, it's the jhipster app generator that wants to keep to read variables with its namespace, when they are stored from yeoman in the blueprint namespace by its default behavior.

Besides the rootGeneratorName is used only in the storage class to save and read configuration by yeoman, changing it the result is only in that class, that not affect the classes of the other subgenerators (server, client, entity...).

mshima commented 4 years ago

The others generators works because of: https://github.com/jhipster/generator-jhipster/blob/7b2f6dd6cac86633879898f3b693f170e9e2e682/generators/client/index.js#L86

But current getAllJhipsterConfig implementation reads blueprints from generator-jhipster namespace. https://github.com/jhipster/generator-jhipster/blob/7b2f6dd6cac86633879898f3b693f170e9e2e682/generators/utils.js#L501

ghost commented 4 years ago

The other generators work because the values read by the getAllJhipsterConfig in the "generator-jhipster" key are defined and stored in the .yo-rc.json by the generator app before running the subgenerators. But if you extend the main app, as I have said before, all is stored under blueprint namespace in the .yo-rc.json (nothing exist under "generator-jhipster" key). So after all will break.

pascalgrimaud commented 4 years ago

closed by https://github.com/jhipster/generator-jhipster/pull/10664

jhipster / generator-jhipster