fix(generate): correct component path when module is generated in subfolder, and parent folder is not a module too (#3916)

fixes #3255

Author: Meligy

packages/angular-cli/blueprints/module/index.js

@@ -58,13 +58,20 @@ module.exports = {
   },
 
   afterInstall: function (options) {
-    // Note that `this.generatePath` already contains `this.dasherizedModuleName`
-    // So, the path will end like `name/name`,
-    //  which is correct for `name.component.ts` created in module `name`
     if (this.options && this.options.routing) {
-      var componentPath = path.join(this.generatePath, this.dasherizedModuleName);
-      options.entity.name = path.relative(this.dynamicPath.appRoot, componentPath);
+
+      // Component folder needs to be `/{moduleName}/{ComponentName}`
+      // Note that we are using `flat`, so no extra dir will be created
+      // We need the leading `/` so the component path resolution work for both cases below:
+      // 1. If module name has no path (no `/`), that's going to be `/mod-name/mod-name`
+      //      as `this.dynamicPath.dir` will be the same as `this.dynamicPath.appRoot`
+      // 2. If it does have `/` (like `parent/mod-name`), it'll be `/parent/mod-name/mod-name`
+      //      as `this.dynamicPath.dir` minus `this.dynamicPath.appRoot` will be `/parent`
+      var moduleDir =
+        this.dynamicPath.dir.replace(this.dynamicPath.appRoot, '') + path.sep + this.dasherizedModuleName;
+      options.entity.name = moduleDir + path.sep + this.dasherizedModuleName;
       options.flat = true;
+
       options.route = false;
       options.inlineTemplate = false;
       options.inlineStyle = false;

tests/acceptance/generate-module.spec.js

@@ -2,9 +2,10 @@
 
 const ng = require('../helpers/ng');
 const tmp = require('../helpers/tmp');
-
+const fs = require('fs-extra');
 const existsSync = require('exists-sync');
 const expect = require('chai').expect;
+const Promise = require('angular-cli/ember-cli/lib/ext/promise');
 const path = require('path');
 const root = process.cwd();
 
@@ -40,7 +41,7 @@ describe('Acceptance: ng generate module', function () {
   });
 
   it('ng generate module  generate routing and component files when passed flag --routing', function () {
-    return ng(['generate', 'module', 'my-module', '--routing']).then( () => {
+    return ng(['generate', 'module', 'my-module', '--routing']).then(() => {
       expect(existsSync(path.join(testPath, 'my-module', 'my-module.module.ts'))).to.equal(true);
       expect(existsSync(path.join(testPath, 'my-module', 'my-module-routing.module.ts'))).to.equal(true);
       expect(existsSync(path.join(testPath, 'my-module', 'my-module.module.spec.ts'))).to.equal(false);
@@ -72,6 +73,35 @@ describe('Acceptance: ng generate module', function () {
     );
   });
 
+  it('ng generate module child should work in sub-dir', function () {
+    fs.mkdirSync(path.join(testPath, './sub-dir'));
+    return new Promise(resolve => {
+      process.chdir(path.join(testPath, './sub-dir'));
+      return resolve();
+    }).then(() =>
+      ng(['generate', 'module', 'child']).then(() => {
+        expect(existsSync(path.join(testPath, 'sub-dir/child', 'child.module.ts'))).to.equal(true);
+        expect(existsSync(path.join(testPath, 'sub-dir/child', 'child.module.spec.ts'))).to.equal(false);
+        expect(existsSync(path.join(testPath, 'sub-dir/child', 'child.component.ts'))).to.equal(false);
+      })
+    );
+  });
+
+  it('ng generate module child should work in sub-dir with routing and component files when passed --routing flag', function () {
+    fs.mkdirSync(path.join(testPath, './sub-dir'));
+    return new Promise(resolve => {
+      process.chdir(path.join(testPath, './sub-dir'));
+      return resolve();
+    }).then(() =>
+      ng(['generate', 'module', 'child', '--routing']).then(() => {
+        expect(existsSync(path.join(testPath, 'sub-dir/child', 'child.module.ts'))).to.equal(true);
+        expect(existsSync(path.join(testPath, 'sub-dir/child', 'child-routing.module.ts'))).to.equal(true);
+        expect(existsSync(path.join(testPath, 'sub-dir/child', 'child.module.spec.ts'))).to.equal(false);
+        expect(existsSync(path.join(testPath, 'sub-dir/child', 'child.component.ts'))).to.equal(true);
+      })
+    );
+  });
+
   it('ng generate module should generate parent/child module with routing and component files when passed --routing flag', function () {
     return ng(['generate', 'module', 'parent']).then(() =>
       ng(['generate', 'module', 'parent/child', '--routing']).then(() => {

tests/e2e/tests/generate/module/module-routing-child-folder.ts

@@ -0,0 +1,33 @@
+import * as fs from 'fs-extra';
+import { join } from 'path';
+import { ng } from '../../../utils/process';
+import { expectFileToExist } from '../../../utils/fs';
+import { expectToFail } from '../../../utils/utils';
+
+
+const Promise = require('angular-cli/ember-cli/lib/ext/promise');
+
+export default function () {
+  const root = process.cwd();
+  const testPath = join(root, 'src', 'app');
+
+  process.chdir(testPath);
+  fs.mkdirSync('./sub-dir');
+
+  return Promise.resolve()
+    .then(() =>
+      ng('generate', 'module', 'sub-dir/child', '--routing')
+        .then(() => expectFileToExist(join(testPath, 'sub-dir/child')))
+        .then(() => expectFileToExist(join(testPath, 'sub-dir/child', 'child.module.ts')))
+        .then(() => expectFileToExist(join(testPath, 'sub-dir/child', 'child-routing.module.ts')))
+        .then(() => expectFileToExist(join(testPath, 'sub-dir/child', 'child.component.ts')))
+        .then(() => expectFileToExist(join(testPath, 'sub-dir/child', 'child.component.spec.ts')))
+        .then(() => expectFileToExist(join(testPath, 'sub-dir/child', 'child.component.html')))
+        .then(() => expectFileToExist(join(testPath, 'sub-dir/child', 'child.component.css')))
+        .then(() => expectToFail(() =>
+          expectFileToExist(join(testPath, 'sub-dir/child', 'child.spec.ts'))
+        ))
+        // Try to run the unit tests.
+        .then(() => ng('test', '--single-run'))
+    );
+}