Repository: ember-cli/ember-cli
Pull request #6224 (2016-09-02 07:00:48)

consolidate package.json normalization
I noticed when generating an in-repo addon, the terminating new line was getting removed from `package.json`. This bug was fixed in addon generation, so the code was consolidated so everyone gets the new line. Then I added tests.

nathanhammond: > "Ugh, this is annoying." /@kellyselden goes and fixes it.

Awesome. My only comment at first pass is that this isn't limited to `package.json` ... it's really any JSON file. So let's name it something different?
kellyselden: @nathanhammond How about `stringifyAndNormalize`?
nathanhammond: `stringifyAndNormalize`: 👍 

Patch:
diff --git a/blueprints/addon/index.js b/blueprints/addon/index.js
index 9dbe4005b..720358e26 100644
--- a/blueprints/addon/index.js
+++ b/blueprints/addon/index.js
@@ -8,6 +8,7 @@ var SilentError = require('silent-error');
 var date        = new Date();
 
 var normalizeEntityName = require('ember-cli-normalize-entity-name');
+var stringifyAndNormalize = require('../../lib/utilities/stringify-and-normalize');
 
 module.exports = {
   description: 'The default blueprint for ember-cli addons.',
@@ -167,5 +168,7 @@ function alphabetizeObjectKeys(unordered) {
 }
 
 function writeContentsToFile(contents, fileName) {
-  fs.writeFileSync(path.join(this.path, 'files', fileName), JSON.stringify(contents, null, 2) + '\n');
+  var packagePath = path.join(this.path, 'files', fileName);
+
+  fs.writeFileSync(packagePath, stringifyAndNormalize(contents));
 }
diff --git a/blueprints/in-repo-addon/index.js b/blueprints/in-repo-addon/index.js
index d26aadf93..74fe18928 100755
--- a/blueprints/in-repo-addon/index.js
+++ b/blueprints/in-repo-addon/index.js
@@ -2,6 +2,7 @@ var fs   = require('fs-extra');
 var path = require('path');
 var stringUtil = require('ember-cli-string-utils');
 var Blueprint = require('../../lib/models/blueprint');
+var stringifyAndNormalize = require('../../lib/utilities/stringify-and-normalize');
 
 module.exports = {
   description: 'The blueprint for addon in repo ember-cli addons.',
@@ -17,41 +18,37 @@ module.exports = {
   },
 
   afterInstall: function(options) {
-    var packagePath = path.join(this.project.root, 'package.json');
-    var contents    = fs.readJsonSync(packagePath);
-    var name        = stringUtil.dasherize(options.entity.name);
-    var newPath     = ['lib', name].join('/');
-    var paths;
-
-    contents['ember-addon'] = contents['ember-addon'] || {};
-    paths = contents['ember-addon']['paths'] = contents['ember-addon']['paths'] || [];
-
-    if (paths.indexOf(newPath) === -1) {
-      paths.push(newPath);
-    }
-
-    fs.writeFileSync(packagePath, JSON.stringify(contents, null, 2));
+    this._generatePackageJson(options, true);
   },
 
   afterUninstall: function(options) {
+    this._generatePackageJson(options, false);
+  },
+
+  _generatePackageJson: function(options, isInstall) {
     var packagePath = path.join(this.project.root, 'package.json');
     var contents    = fs.readJsonSync(packagePath);
     var name        = stringUtil.dasherize(options.entity.name);
     var newPath     = ['lib', name].join('/');
     var paths;
-    var newPathIndex;
 
     contents['ember-addon'] = contents['ember-addon'] || {};
     paths = contents['ember-addon']['paths'] = contents['ember-addon']['paths'] || [];
-    newPathIndex = paths.indexOf(newPath);
 
-    if (newPathIndex > -1) {
-      paths.splice(newPathIndex, 1);
-      if (paths.length === 0) {
-        delete contents['ember-addon']['paths'];
+    if (isInstall) {
+      if (paths.indexOf(newPath) === -1) {
+        paths.push(newPath);
+      }
+    } else {
+      var newPathIndex = paths.indexOf(newPath);
+      if (newPathIndex > -1) {
+        paths.splice(newPathIndex, 1);
+        if (paths.length === 0) {
+          delete contents['ember-addon']['paths'];
+        }
       }
     }
 
-    fs.writeFileSync(packagePath, JSON.stringify(contents, null, 2));
+    fs.writeFileSync(packagePath, stringifyAndNormalize(contents));
   }
 };
diff --git a/lib/utilities/stringify-and-normalize.js b/lib/utilities/stringify-and-normalize.js
new file mode 100644
index 000000000..c0fdba5f3
--- /dev/null
+++ b/lib/utilities/stringify-and-normalize.js
@@ -0,0 +1,5 @@
+'use strict';
+
+module.exports = function stringifyAndNormalize(contents) {
+  return JSON.stringify(contents, null, 2) + '\n';
+};


Test patch:
diff --git a/tests/unit/blueprints/addon-test.js b/tests/unit/blueprints/addon-test.js
index 567ad2d92..f96ba1df2 100644
--- a/tests/unit/blueprints/addon-test.js
+++ b/tests/unit/blueprints/addon-test.js
@@ -91,7 +91,7 @@ describe('blueprint - addon', function() {
         td.verify(writeFileSync(path.normalize('test-blueprint-path/files/package.json'), captor.capture()));
 
         // string to test ordering
-        expect(captor.value).to.deep.equal('\
+        expect(captor.value).to.equal('\
 {\n\
   "name": "test-project-name",\n\
   "description": "The default blueprint for ember-cli addons.",\n\
@@ -274,18 +274,6 @@ describe('blueprint - addon', function() {
         expect(json.dependencies).to.deep.equal({ a: "1", b: "1" });
         expect(json.devDependencies).to.deep.equal({ a: "1", b: "1" });
       });
-
-      it('appends ending newline', function() {
-        blueprint.generatePackageJson();
-
-        var captor = td.matchers.captor();
-
-        td.verify(readJsonSync(path.normalize('test-app-blueprint-path/files/package.json')));
-        td.verify(writeFileSync(path.normalize('test-blueprint-path/files/package.json'), captor.capture()));
-
-        var contents = captor.value;
-        expect(contents[contents.length - 1]).to.equal('\n');
-      });
     });
 
     describe('generateBowerJson', function() {
@@ -298,23 +286,11 @@ describe('blueprint - addon', function() {
         td.verify(writeFileSync(path.normalize('test-blueprint-path/files/bower.json'), captor.capture()));
 
         // string to test ordering
-        expect(captor.value).to.deep.equal('\
+        expect(captor.value).to.equal('\
 {\n\
   "name": "test-project-name"\n\
 }\n');
       });
-
-      it('appends ending newline', function() {
-        blueprint.generateBowerJson();
-
-        var captor = td.matchers.captor();
-
-        td.verify(readJsonSync(path.normalize('test-app-blueprint-path/files/bower.json')));
-        td.verify(writeFileSync(path.normalize('test-blueprint-path/files/bower.json'), captor.capture()));
-
-        var contents = captor.value;
-        expect(contents[contents.length - 1]).to.equal('\n');
-      });
     });
   });
 });
diff --git a/tests/unit/blueprints/in-repo-addon-test.js b/tests/unit/blueprints/in-repo-addon-test.js
index aa18a8b75..ab8d8c889 100644
--- a/tests/unit/blueprints/in-repo-addon-test.js
+++ b/tests/unit/blueprints/in-repo-addon-test.js
@@ -7,6 +7,8 @@ var setupTestHooks = blueprintHelpers.setupTestHooks;
 var emberNew = blueprintHelpers.emberNew;
 var emberGenerate = blueprintHelpers.emberGenerate;
 var emberDestroy = blueprintHelpers.emberDestroy;
+var proxyquire = require('proxyquire');
+var td = require('testdouble');
 
 var expect = require('ember-cli-blueprint-test-helpers/chai').expect;
 var file = require('ember-cli-blueprint-test-helpers/chai').file;
@@ -54,3 +56,122 @@ describe('Acceptance: ember generate and destroy in-repo-addon', function() {
       });
   });
 });
+
+describe('Unit: in-repo-addon blueprint', function() {
+  var blueprint;
+  var readJsonSync;
+  var writeFileSync;
+  var options;
+
+  beforeEach(function() {
+    readJsonSync = td.function();
+    writeFileSync = td.function();
+
+    blueprint = proxyquire('../../../blueprints/in-repo-addon', {
+      'fs-extra': {
+        readJsonSync: readJsonSync,
+        writeFileSync: writeFileSync,
+      }
+    });
+    blueprint.project = {
+      root: 'test-project-root'
+    };
+
+    options = {
+      entity: {
+        name: 'test-entity-name'
+      }
+    }
+  });
+
+  it('adds to paths', function() {
+    td.when(readJsonSync(), { ignoreExtraArgs: true }).thenReturn({});
+
+    blueprint.afterInstall(options);
+
+    var captor = td.matchers.captor();
+
+    td.verify(readJsonSync(path.normalize('test-project-root/package.json')));
+    td.verify(writeFileSync(path.normalize('test-project-root/package.json'), captor.capture()));
+
+    expect(captor.value).to.equal('\
+{\n\
+  "ember-addon": {\n\
+    "paths": [\n\
+      "lib/test-entity-name"\n\
+    ]\n\
+  }\n\
+}\n');
+  });
+
+  it('ignores if already exists', function() {
+    td.when(readJsonSync(), { ignoreExtraArgs: true }).thenReturn({
+      'ember-addon': {
+        paths: ['lib/test-entity-name']
+      }
+    });
+
+    blueprint.afterInstall(options);
+
+    var captor = td.matchers.captor();
+
+    td.verify(readJsonSync(path.normalize('test-project-root/package.json')));
+    td.verify(writeFileSync(path.normalize('test-project-root/package.json'), captor.capture()));
+
+    expect(captor.value).to.equal('\
+{\n\
+  "ember-addon": {\n\
+    "paths": [\n\
+      "lib/test-entity-name"\n\
+    ]\n\
+  }\n\
+}\n');
+  });
+
+  it('removes from paths', function() {
+    td.when(readJsonSync(), { ignoreExtraArgs: true }).thenReturn({
+      'ember-addon': {
+        paths: [
+          'lib/test-entity-name',
+          'lib/test-entity-name-2'
+        ]
+      }
+    });
+
+    blueprint.afterUninstall(options);
+
+    var captor = td.matchers.captor();
+
+    td.verify(readJsonSync(path.normalize('test-project-root/package.json')));
+    td.verify(writeFileSync(path.normalize('test-project-root/package.json'), captor.capture()));
+
+    expect(captor.value).to.equal('\
+{\n\
+  "ember-addon": {\n\
+    "paths": [\n\
+      "lib/test-entity-name-2"\n\
+    ]\n\
+  }\n\
+}\n');
+  });
+
+  it('removes paths if last one', function() {
+    td.when(readJsonSync(), { ignoreExtraArgs: true }).thenReturn({
+      'ember-addon': {
+        paths: ['lib/test-entity-name']
+      }
+    });
+
+    blueprint.afterUninstall(options);
+
+    var captor = td.matchers.captor();
+
+    td.verify(readJsonSync(path.normalize('test-project-root/package.json')));
+    td.verify(writeFileSync(path.normalize('test-project-root/package.json'), captor.capture()));
+
+    expect(captor.value).to.equal('\
+{\n\
+  "ember-addon": {}\n\
+}\n');
+  });
+});
diff --git a/tests/unit/utilities/package-json-stringify-test.js b/tests/unit/utilities/package-json-stringify-test.js
new file mode 100644
index 000000000..3aba3a4db
--- /dev/null
+++ b/tests/unit/utilities/package-json-stringify-test.js
@@ -0,0 +1,23 @@
+'use strict';
+
+var expect                = require('chai').expect;
+var stringifyAndNormalize = require('../../../lib/utilities/stringify-and-normalize');
+
+describe('package-json-stringify', function() {
+  var packageJson = {
+    dependencies: {
+      'test-package': '^1.0.0'
+    }
+  };
+
+  it('indents 2 spaces and ends in newline', function() {
+    var string = stringifyAndNormalize(packageJson);
+
+    expect(string).to.equal('\
+{\n\
+  "dependencies": {\n\
+    "test-package": "^1.0.0"\n\
+  }\n\
+}\n');
+  });
+});