Feature Request : permit to configure directive and component prefix by module

[nweldev]

OS?

All

Versions.

angular-cli: 1.0.0-beta.22-1
node: 6.9.1
os: win32 x64

Why?

Using different components and directives prefixes for each module :

1. improve readability (exemple : 'hero-info' instead of 'app-hero-info').
2. ease moving / copying an existing module to a separate application (it shouldn't require to change all components and directive prefixes).

Bonus : I would be glad to help to implement that.

Use case

ng new cli-22-1-test
cd cli-22-1-test
ng g m hero

Now, generate a new component : ng g c hero/info.

Generated hero/info/info.component.ts :

import { Component, OnInit } from '@angular/core';

@Component({
  selector: 'app-info',
  templateUrl: './info.component.html',
  styleUrls: ['./info.component.css']
})
export class InfoComponent implements OnInit {

  constructor() { }

  ngOnInit() {
  }

}

Here, the selector should be "hero-info" instead of "app-info".

Extra

Generated hero/hero-component.ts :

import { Component, OnInit } from '@angular/core';

@Component({
  selector: 'app-hero',
  templateUrl: './hero.component.html',
  styleUrls: ['./hero.component.css']
})
export class HeroComponent implements OnInit {

  constructor() { }

  ngOnInit() {
  }

}

Here, the selector could be "hero-root" instead of "app-hero".

Option 1

Permit to override the root angular-cli.json file with an angular-cli.json file per module.

Following the previous example, we would create a src/app/hero/angular-cli.json file :

{
  "apps": [
    {
      "prefix": "hero"
    }
  ]
}

Of course, only few options should be available.

Option 2

Add a apps[0].modules config parameter for that.

Example :

{
  "project": {
    "version": "1.0.0-beta.22-1",
    "name": "cli-22-1"
  },
  "apps": [
    {
      ...
      modules: {
          "hero" : {
              "prefix": "hero"
           }
      }
    }
  ],
  "addons": [],
  "packages": [],
  ...
  }
}

Option 3

Change apps[0].prefix: string to :

prefixes: {
     "default": "app",
     "hero": "hero"
}

[clydin]

A --prefix option on the generate module command that would store the module and prefix info so that it could be used in subsequent generate commands within the module's hierarchy could be a nice addition.

[Brocco]

I like this idea, just not totally sold on the configuration options you've thrown out thus far. The config file angular-cli.json has gotten quite busy of recent and I'd like to keep this out of there while making the module more portable.

What do you think about the idea of adding an additional export to the NgModule file which could look like this:

const ModulePrefix = 'hero';

Then when components are generated it looks for a module file, and searches the AST for that file for a variable named ModulePrefix which can be parsed out and used to construct the selector for the new component.

Thoughts?

[nweldev]

I think this idea is by far the best option ! If you have some directions / advices to give, it would allow me to do it faster.

Edit : I think I found enough elements on my own to start the associated development today.

[nweldev]

Ok so, I tested the concept with a require(pathToModule), and everithing seems ok. I think we will need to add a --prefix option for 'generate module' after that, and use a ${prefix}-root selector for the main component, along with the generated "ModulePrefix" var.

Following that, I'm having a hard time to find how to "searches the AST for that file for a variable named ModulePrefix" in order to replace this "require".

I didn't found any method to do this kind of stuff in the ast-tools or utilities, and, given that i'm just a beginner with the ts api, all I could do, based on getImportClauses, was :

astUtils.getSourceNodes(astUtils.getSource(pathToModule))
        .filter(node => (node.flags & ts.NodeFlags.Export) !== 0)
        .subscribe(node => console.log(node.getText()));

I didn't find any TS API documentation except this doc and this types definitions.

So, my question is : how could i get the identifier (to test if equal to "ModulePrefix") and the initializer (to have the actual value that I need) for a Node like export const ModulePrefix = 'hero' ?

[baruchvlz]

This is how some of the generate tests verify content inside the file created.

Example for ng generate module:

var fs = require('fs-extra');
const denodeify = require('denodeify');
const readFile = denodeify(fs.readFile);

let testPath = path.join(root, 'tmp', 'foo', 'src', 'app', 'a-test', 'a-test.module.ts');
  return ng(['generate', 'module', 'a-test-module'])
    .then(() => {
      expect(existsSync(testPath)).to.equal(true);
    })
    .then(() => readFile(testPath, 'utf-8'))
    .then(content => {
      expect(content).to.matches(/^export\sclass\s(ATestModule)/m);
    });

You could probably use the same concept to test if ModulePrefix exists.

Edit: ModulePrefix should be either modulePrefix or MODULE_PREFIX to follow the Style Guide

[nweldev]

Thanks @baruchvlz. So, what you say is that the following solution is ok ?

astUtils.getSourceNodes(astUtils.getSource(pathToModule))
        .last(node => (node.flags & ts.NodeFlags.Export) !== 0 && node.getText().indexOf('ModulePrefix') > -1)
        .subscribe(node => {
          modulePrefix = /= ?['"]([\w-]+)["']/.exec(node.getText())[1];
        });

I'm not really satisfied by that :/

Edit : the expected behaviour is now ok with this code, but I still need to write the associated UT, and I think it would be better if I moved this "getExportedConstValue" to a new utility.

[clydin]

I like the idea of storing the information in the NgModule's file but it seems almost wrong to mix executable code with CLI metadata.
What about using comment tags instead (similar to tslint for instance). Something like "/* angular-cli:module-prefix:hero */"

This idea could also be extended for other use cases in the future as well.

[nweldev]

@clydin, I think it could be a good idea.

But this kind of decision could be really important for later : do we decide to add extra informations for angular-cli by comment or code ?

Anyway, we'll need to make utilities for that in order to ease this kind of behaviour in the futur :

• with the first option (exported constant - Feature Request : permit to configure directive and component prefix by module #3452 (comment)), this could only be a "utilities/get-config-node" file with a single function
• but the second option (Feature Request : permit to configure directive and component prefix by module #3452 (comment)) should lead to the creation of a far more complexe solution
  This is why I propose to resolve this issue with the first option at first, and then, create a new feature request "override configurations by comments".

So, I'll create the associated commit in a few minutes, but it's not ready yet (see #3452 (comment)), and is based on #3452 (comment).

I'll also create another feature request about #3452 (comment) later, in order to keep the scope of this one as lite as possible.

[hansl]

I'm confused on this issue. @Brocco If you want per-module prefix, why not just figure out the prefix from the module name itself (based on an option)? I don't like using exports from files as configuration, because 1) it's not centralized, 2) you will end up with configuration in your production app and 3) it's not standard and won't be respected by other tools. It's just weird.

Modules are not libraries, they're just organization for your app/library. The prefix you use should be cross-modules (like we do in Material where we have 1 module per component but a single prefix). One prefix for your library, one prefix for your app. No need for a prefix for each module...

It comes down to how opinionated we really want to be. I agree the angular-cli.json is getting larger than what it needs, but there are other options than using code to expose configuration to tooling.

(PS: lint options are so tightly coupled to single files that it makes sense to have comments for them. I put them on a different scale than our own options.)

[hansl]

I'm for Options 1 myself; having angular-cli.json files in a directory would have the flexibility without having to ship anything. At this point though I'd like to change the name of the configuration (like just .angular.json or .angular.config.json or something similar).

[clydin]

@hansl, I agree that options in general don't make sense as comments. However, in this case the goal was to attach metadata to a NgModule definition. With the TS AST's capability to retrieve comments that are "owned" by a node (here the NgModule's class), it seemed like a nice fit; if the goal was to store the info in the file and to ensure it would be removed when minified for production.

But I'm in no way attached to the idea. Option 1 is probably the most flexible. My concern would be an explosion of angular-cli.json files. In a large project, having a large amount of files with the same name can cause housekeeping issues.

[hansl]

I was referring to Mike's idea of exporting or having a local variable in the code. Comment is slightly better, but still not a proper solution. I don't mind comment configuration if they are pertinent to the file itself, but in this case it's more of a "this collection of files have this special configuration exception", which I think a file is better.

[Brocco]

@hansl I can see your argument and can concede to the idea of not having the configuration of the module specific prefix within the module file.

[nweldev]

Oh snap, this IS an old and deprecated issue ! Now that we have schematics, ng-packagr, devkit, nrwl/nx, ng g library, etc ... I don't think this issue is still relevant, as prefix (and all other cli configurations) should be associated to an app / project / lib, not a module, since we now can easily move those modules to a separated library project. @Brocco, if you think per module configuration is still relevant, feel free to reopen and I'll join the conversation 😉

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}