Merge pull request #119 from facebook/collapsed-objects

Collapse duplicate objects in lockfile to single line

Author: Sebastian McKenzie

src/lockfile/index.js

@@ -114,7 +114,8 @@ export default class Lockfile {
     [packagePattern: string]: Manifest
   }): Object {
     let lockfile = {};
-    let seen: Map<string, string> = new Map;
+    let seen: Map<string, Object> = new Map;
+
     // order by name so that lockfile manifest is assigned to the first dependency with this manifest
     // the others that have the same remote.resovled will just refer to the first
     // oredring allows for consistency in lockfile when it is serialized
@@ -138,7 +139,7 @@ export default class Lockfile {
 
       invariant(remote, "Package is missing a remote");
 
-      lockfile[pattern] = {
+      let obj = {
         name: pkg.name,
         version: pkg.version,
         uid: pkg.uid === pkg.version ? undefined : pkg.uid,
@@ -148,9 +149,10 @@ export default class Lockfile {
         optionalDependencies: _.isEmpty(pkg.optionalDependencies) ? undefined : pkg.optionalDependencies,
         permissions: _.isEmpty(ref.permissions) ? undefined : ref.permissions
       };
+      lockfile[pattern] = obj;
 
       if (remote.resolved) {
-        seen.set(remote.resolved, pattern);
+        seen.set(remote.resolved, obj);
       }
     }
 

src/lockfile/parse.js

@@ -24,11 +24,16 @@ const TOKEN_TYPES = {
   newline: "NEWLINE",
   indent: "INDENT",
   invalid: "INVALID",
-  number: "NUMBER"
+  number: "NUMBER",
+  comma: "COMMA"
 };
 
 const VALID_PROP_VALUE_TOKENS = [TOKEN_TYPES.boolean, TOKEN_TYPES.string, TOKEN_TYPES.number];
 
+function isValidPropValueToken(token): boolean {
+  return VALID_PROP_VALUE_TOKENS.indexOf(token.type) >= 0;
+}
+
 type Token = {
   line: number,
   col: number,
@@ -111,6 +116,9 @@ export function* tokenise(input: string): Iterator<Token> {
     } else if (input[0] === ":") {
       yield buildToken(TOKEN_TYPES.colon);
       chop++;
+    } else if (input[0] === ",") {
+      yield buildToken(TOKEN_TYPES.comma);
+      chop++;
     } else if (/^[a-zA-Z]/g.test(input)) {
       let name = "";
       for (let i = 0; i < input.length; i++) {
@@ -172,6 +180,15 @@ export class Parser {
     }
   }
 
+  eat(tokType: string): boolean {
+    if (this.token.type === tokType) {
+      this.next();
+      return true;
+    } else {
+      return false;
+    }
+  }
+
   parse(indent: number = 0): Object {
     let obj = map();
 
@@ -210,16 +227,37 @@ export class Parser {
         let key = propToken.value;
         invariant(key, "Expected a key");
 
+        let keys = [key];
+
+        // support multiple keys
+        while (this.eat(TOKEN_TYPES.comma)) {
+          let keyToken = this.token;
+          this.expect(TOKEN_TYPES.string);
+
+          let key = keyToken.value;
+          invariant(key, "Expected a key");
+          keys.push(key);
+        }
+
         let valToken = this.next();
-        if (valToken.type === TOKEN_TYPES.colon) {
+        if (valToken.type === TOKEN_TYPES.colon) { // object
           this.next();
-          obj[key] = this.parse(indent + 1);
+
+          // parse object
+          let val = this.parse(indent + 1);
+
+          for (let key of keys) {
+            obj[key] = val;
+          }
 
           if (indent && this.token.type !== TOKEN_TYPES.indent) {
             break;
           }
-        } else if (VALID_PROP_VALUE_TOKENS.indexOf(valToken.type) >= 0) {
-          obj[key] = valToken.value;
+        } else if (isValidPropValueToken(valToken)) { // plain value
+          for (let key of keys) {
+            obj[key] = valToken.value;
+          }
+
           this.next();
         } else {
           this.unexpected("Invalid value type");

src/lockfile/stringify.js

@@ -13,7 +13,7 @@ let _ = require("lodash");
 
 function shouldWrapKey(str: string): boolean {
   return str.indexOf("true") === 0 || str.indexOf("false") === 0 ||
-         /[:\s\n\\"]/g.test(str) || /^[0-9]/g.test(str) || !/^[a-zA-Z]/g.test(str);
+         /[:\s\n\\",]/g.test(str) || /^[0-9]/g.test(str) || !/^[a-zA-Z]/g.test(str);
 }
 
 function maybeWrap(str: string): string {
@@ -56,19 +56,39 @@ export default function stringify(obj: any, indent: string = ""): string {
   // stable sort, V8 Array.prototype.sort is not stable and we don't want to shuffle things randomly
   keys = _.sortBy(keys, getKeyPriority);
 
-  for (let key of keys) {
+  let addedKeys = [];
+
+  for (let i = 0; i < keys.length; i++) {
+    let key = keys[i];
     let val = obj[key];
     if (val == null) continue;
+    if (addedKeys.indexOf(key) >= 0) continue;
+
+    //
+    let valKeys = [key];
 
-    key = maybeWrap(key);
+    // get all keys that have the same value equality, we only want this for objects
+    if (typeof val === "object") {
+      for (let j = i + 1; j < keys.length; j++) {
+        let key = keys[j];
+        if (val === obj[key]) {
+          valKeys.push(key);
+        }
+      }
+    }
+
+    //
+    let keyLine = valKeys.map(maybeWrap).join(", ");
 
     if (typeof val === "string" || typeof val === "boolean" || typeof val === "number") {
-      lines.push(`${key} ${maybeWrap(val)}`);
+      lines.push(`${keyLine} ${maybeWrap(val)}`);
     } else if (typeof val === "object") {
-      lines.push(`${key}:\n${stringify(val, indent + "  ")}`);
+      lines.push(`${keyLine}:\n${stringify(val, indent + "  ")}`);
     } else {
       throw new TypeError;
     }
+
+    addedKeys = addedKeys.concat(valKeys);
   }
 
   return indent + lines.join(`\n${indent}`);

test/lockfile.js

@@ -51,8 +51,8 @@ test("parse", (t) => {
 });
 
 test("stringify", (t) => {
-  stringify;
-  t;
+  let obj = { foo: "bar" };
+  t.deepEqual(stringify({ a: obj, b: obj }), "a, b:\n  foo bar");
 });
 
 test("Lockfile.isStrict", (t) => {
@@ -147,17 +147,19 @@ test("Lockfile.getLockfile", (t) => {
 
   let actual = new Lockfile().getLockfile(patterns);
 
+  let expectedFoobar = {
+    name: "foobar",
+    version: "0.0.0",
+    uid: undefined,
+    resolved: "http://example.com/foobar",
+    registry: undefined,
+    dependencies: undefined,
+    optionalDependencies: undefined,
+    permissions: undefined
+  };
+
   let expected = {
-    foobar: {
-      name: "foobar",
-      version: "0.0.0",
-      uid: undefined,
-      resolved: "http://example.com/foobar",
-      registry: undefined,
-      dependencies: undefined,
-      optionalDependencies: undefined,
-      permissions: undefined
-    },
+    foobar: expectedFoobar,
 
     barfoo: {
       name: "barfoo",
@@ -170,7 +172,7 @@ test("Lockfile.getLockfile", (t) => {
       permissions: { foo: "bar" }
     },
 
-    foobar2: "foobar"
+    foobar2: expectedFoobar
   };
 
   t.deepEqual(actual, expected);
@@ -198,19 +200,20 @@ test("Lockfile.getLockfile (sorting)", (t) => {
 
   let actual = new Lockfile().getLockfile(patterns);
 
-  let expected = {
-    foobar1: {
-      name: "foobar",
-      version: "0.0.0",
-      uid: undefined,
-      resolved: "http://example.com/foobar",
-      registry: undefined,
-      dependencies: undefined,
-      optionalDependencies: undefined,
-      permissions: undefined
-    },
+  let expectedFoobar = {
+    name: "foobar",
+    version: "0.0.0",
+    uid: undefined,
+    resolved: "http://example.com/foobar",
+    registry: undefined,
+    dependencies: undefined,
+    optionalDependencies: undefined,
+    permissions: undefined
+  };
 
-    foobar2: "foobar1"
+  let expected = {
+    foobar1: expectedFoobar,
+    foobar2: expectedFoobar
   };
 
   t.deepEqual(actual, expected);