Repository: opencomponents/oc (js)

Issue:
oc package <componentPath> does not find modules in componentPath
## *Who* is the bug affecting?
component creators

## *What* is affected by this bug?
oc cli tool

## *When* does this occur?
When packaging component

## *Where* on the platform does it happen?
cli


## *How* do we replicate the issue?
- npm install -g oc@0.44.13
- Create an OC component (e.g. temp-oc)
- npm install in temp-oc root
- npm build temp-oc
- cd ..
- oc package ./temp-oc
- see the error
```
Packaging -> /Users/x/components/temp-oc/_package
Ensuring dependencies are loaded...
Trying to install missing modules: foo@^1.3.21, bar@^0.1.0
If you aren't connected to the internet, or npm isn't configured then this step will fail...FAIL
```

Note: `foo` and `bar` are installed through internal npm artifact


### working example
- npm install -g oc@0.44.13
- Create an OC component (e.g. temp-oc)
- npm install in temp-oc root
- npm build temp-oc
- oc package .
- output:
```
Packaging -> /Users/x/components/temp-oc/_package
Ensuring dependencies are loaded...
Packaged -> /Users/x/components/temp-oc/_package
```

## Expected behavior (i.e. solution)
oc package <componentPath> should respect node_modules instaled in `componentPath/node_modules`

## What version of OC, Node.js and OS are you using?
oc@0.44.13, Node v8.11.1, Mac

## Other Comments
possibly problem comes from this line
https://github.com/opencomponents/oc/blob/efeadfbfda5413080e7e908f112e20c71bb9f311/src/cli/facade/package.js#L21

Pull request:
oc package now is able to reuse modules in component path
**Closing issues**
closes #943

**What existing problem does the pull request solve?**

A new parameter 'useComponentDependencies' being introduced to package command that make oc package command able to reuse already installed modules in component directory instead of reinstall them. Reinstalling missing modules makes more network call and makes the release process longer also if missing modules are stored in internal npm artifact with required credential to retrieve them then package command will fail without clear understanding of the issue

**New function linkMissingDependencies**

Similar to `installMissingDependencies`, `linkMissingDependencies` will try to create a symbolic link by **fs symlink** to reference the missing dependency to OC's dependency in node_modules folder

**Test plan (required)**

As described in #943, now this scenario works without throwing error

- Create an OC component (e.g. temp-oc)
- npm install in temp-oc root
- npm build temp-oc
- cd ..
- oc package ./temp-oc
- and output is 
```
Packaging -> /Users/x/components/temp-oc/_package
Ensuring dependencies are loaded...
Packaged -> /Users/x/components/temp-oc/_package
```

Patch:
diff --git a/src/cli/commands.js b/src/cli/commands.js
index 531940ac..acf0507f 100644
--- a/src/cli/commands.js
+++ b/src/cli/commands.js
@@ -102,6 +102,12 @@ module.exports = {
           boolean: true,
           description: 'Create zipped file',
           default: false
+        },
+        useComponentDependencies: {
+          boolean: true,
+          description:
+            'Reuse already installed dependencies to save packaging time and network bandwidth',
+          default: false
         }
       },
       description: 'Create the packaged component ready to be published',
diff --git a/src/cli/domain/handle-dependencies/index.js b/src/cli/domain/handle-dependencies/index.js
index d75bc2b3..e19d933d 100644
--- a/src/cli/domain/handle-dependencies/index.js
+++ b/src/cli/domain/handle-dependencies/index.js
@@ -9,6 +9,7 @@ const _ = require('lodash');
 const ensureCompilerIsDeclaredAsDevDependency = require('./ensure-compiler-is-declared-as-devDependency');
 const getCompiler = require('./get-compiler');
 const installMissingDependencies = require('./install-missing-dependencies');
+const linkMissingDependencies = require('./link-missing-dependencies');
 const isTemplateLegacy = require('../../../utils/is-template-legacy');
 const strings = require('../../../resources');
 
@@ -16,7 +17,7 @@ const getComponentPackageJson = (componentPath, cb) =>
   fs.readJson(path.join(componentPath, 'package.json'), cb);
 
 module.exports = (options, callback) => {
-  const { components, logger } = options;
+  const { components, logger, useComponentDependencies } = options;
 
   const dependencies = {};
   const addDependencies = componentDependencies =>
@@ -73,8 +74,11 @@ module.exports = (options, callback) => {
       modules: _.union(coreModules, _.keys(dependencies)).sort(),
       templates: _.values(templates)
     };
-
-    const installOptions = { dependencies, logger };
-    installMissingDependencies(installOptions, err => callback(err, result));
+    const options = { dependencies, logger };
+    if (useComponentDependencies) {
+      options.componentPath = components[0];
+      return linkMissingDependencies(options, err => callback(err, result));
+    }
+    installMissingDependencies(options, err => callback(err, result));
   });
 };
diff --git a/src/cli/domain/handle-dependencies/link-missing-dependencies.js b/src/cli/domain/handle-dependencies/link-missing-dependencies.js
new file mode 100644
index 00000000..dc55949c
--- /dev/null
+++ b/src/cli/domain/handle-dependencies/link-missing-dependencies.js
@@ -0,0 +1,51 @@
+'use strict';
+
+const format = require('stringformat');
+const _ = require('lodash');
+const path = require('path');
+const fs = require('fs-extra');
+const getMissingDependencies = require('./get-missing-dependencies');
+const strings = require('../../../resources/index');
+
+const getModuleName = dependency => dependency.split('@')[0];
+
+module.exports = (options, callback) => {
+  const { componentPath, dependencies, logger } = options;
+
+  const missingDependencies = getMissingDependencies(dependencies);
+
+  if (_.isEmpty(missingDependencies)) {
+    return callback(null);
+  }
+
+  logger.warn(
+    format(
+      strings.messages.cli.LINKING_DEPENDENCIES,
+      missingDependencies.join(', ')
+    ),
+    true
+  );
+
+  const symLinkType = 'dir';
+  let symLinkError = false;
+  _.each(missingDependencies, dependency => {
+    const moduleName = getModuleName(dependency);
+    const pathToComponentModule = path.resolve(
+      componentPath,
+      'node_modules',
+      moduleName
+    );
+    const pathToModule = path.resolve('.', 'node_modules', moduleName);
+    try {
+      fs.ensureSymlinkSync(pathToComponentModule, pathToModule, symLinkType);
+    } catch (err) {
+      symLinkError = true;
+      logger.err(
+        format(strings.errors.cli.DEPENDENCY_LINK_FAIL, moduleName, err)
+      );
+    }
+  });
+  return !symLinkError
+    ? callback(null)
+    : callback(strings.errors.cli.DEPENDENCIES_LINK_FAIL);
+};
diff --git a/src/cli/facade/package.js b/src/cli/facade/package.js
index 5b10fcbf..1e79a683 100644
--- a/src/cli/facade/package.js
+++ b/src/cli/facade/package.js
@@ -12,6 +12,7 @@ module.exports = function(dependencies) {
 
   return function(opts, callback) {
     const componentPath = opts.componentPath,
+      useComponentDependencies = opts.useComponentDependencies,
       packageDir = path.resolve(componentPath, '_package'),
       compressedPackagePath = path.resolve(componentPath, 'package.tar.gz');
 
@@ -19,7 +20,11 @@ module.exports = function(dependencies) {
 
     logger.warn(format(strings.messages.cli.PACKAGING, packageDir));
     handleDependencies(
-      { components: [path.resolve(componentPath)], logger },
+      {
+        components: [path.resolve(componentPath)],
+        logger,
+        useComponentDependencies
+      },
       (err, dependencies) => {
         if (err) {
           logger.err(err);
diff --git a/src/resources/index.js b/src/resources/index.js
index cba6b2ed..0f27a787 100644
--- a/src/resources/index.js
+++ b/src/resources/index.js
@@ -151,6 +151,9 @@ module.exports = {
       COMPONENTS_NOT_FOUND: 'no components found in specified path',
       DEPENDENCIES_INSTALL_FAIL:
         'An error happened when installing the dependencies',
+      DEPENDENCY_LINK_FAIL:
+        'An error happened when linking the dependency {0} with error {1}',
+      DEPENDENCIES_LINK_FAIL: 'An error happened when linking the dependencies',
       DEV_FAIL: 'An error happened when initialising the dev runner: {0}',
       FOLDER_IS_NOT_A_FOLDER: '"{0}" must be a directory',
       FOLDER_NOT_FOUND: '"{0}" not found',
@@ -219,6 +222,8 @@ module.exports = {
         'OC dev is running with hot reloading disabled so changes will be ignored',
       INSTALLING_DEPS:
         "Trying to install missing modules: {0}\nIf you aren't connected to the internet, or npm isn't configured then this step will fail...",
+      LINKING_DEPENDENCIES:
+        'Trying to link missing modules: {0}\nThe missing dependencies will be linked to component dependencies',
       MOCKED_PLUGIN: 'Mock for plugin has been registered: {0} () => {1}',
       NO_SUCH_COMMAND: "No such command '{0}'",
       NOT_VALID_REGISTRY_COMMAND:
diff --git a/tasks/npm-publish.js b/tasks/npm-publish.js
index 25562765..01b24b28 100644
--- a/tasks/npm-publish.js
+++ b/tasks/npm-publish.js
@@ -9,9 +9,7 @@ const ocVersion = fs.readJsonSync('./package.json').version;
 
 if (builtVersion !== ocVersion) {
   log.fatal(
-    `The oc-client built version (${
-      builtVersion
-    }) doesn't match the npm package version (${ocVersion})`
+    `The oc-client built version (${builtVersion}) doesn't match the npm package version (${ocVersion})`
   );
   process.exit(1);
 }
@@ -27,7 +25,7 @@ cmd.on('error', () => {
 
 cmd.on('close', code => {
   if (code === 0) {
-    log.complete('npm publish succeded');
+    log.complete('npm publish succeeded');
   }
 
   process.exit(code);


Test patch:
diff --git a/test/unit/cli-domain-handle-dependencies-link-missing-dependencies.js b/test/unit/cli-domain-handle-dependencies-link-missing-dependencies.js
new file mode 100644
index 00000000..fd022252
--- /dev/null
+++ b/test/unit/cli-domain-handle-dependencies-link-missing-dependencies.js
@@ -0,0 +1,129 @@
+'use strict';
+
+const expect = require('chai').expect;
+const injectr = require('injectr');
+const sinon = require('sinon');
+
+describe('cli : domain : handle-dependencies : install-missing-dependencies', () => {
+  let error, logger;
+  const initialise = (options, done) => {
+    const { dependencies, stubs } = options;
+    logger = {
+      err: sinon.spy(),
+      ok: sinon.spy(),
+      warn: sinon.spy()
+    };
+    const linkMissingDependencies = injectr(
+      '../../src/cli/domain/handle-dependencies/link-missing-dependencies.js',
+      {
+        './get-missing-dependencies': stubs.getMissingDependencies,
+        'fs-extra': { ensureSymlinkSync: stubs.ensureSymlinkSync },
+        path: { resolve: () => '/path/to/oc-running' }
+      }
+    );
+
+    const installOptions = {
+      componentPath: '/path/to/oc-component',
+      dependencies,
+      logger
+    };
+    linkMissingDependencies(installOptions, err => {
+      error = err;
+      done();
+    });
+  };
+
+  describe('when there is no missing dependency', () => {
+    let dependencies, stubs;
+    beforeEach(done => {
+      stubs = {
+        getMissingDependencies: sinon.stub().returns([]),
+        ensureSymlinkSync: sinon
+          .stub()
+          .onCall()
+          .yields(null)
+      };
+
+      dependencies = { lodash: '1.2.3' };
+      initialise({ dependencies, stubs }, done);
+    });
+
+    it('should return no error', () => {
+      expect(error).to.be.null;
+    });
+
+    it('should not install anything', () => {
+      expect(stubs.ensureSymlinkSync.called).to.be.false;
+    });
+  });
+
+  describe('when there are missing dependencies and link succeeds', () => {
+    let dependencies, stubs;
+
+    beforeEach(done => {
+      stubs = {
+        getMissingDependencies: sinon.stub(),
+        ensureSymlinkSync: sinon
+          .stub()
+          .onCall()
+          .yields(null)
+      };
+
+      stubs.getMissingDependencies
+        .onCall(0)
+        .returns(['lodash@1.2.3', 'underscore@latest']);
+
+      dependencies = { lodash: '1.2.3', underscore: '' };
+      initialise({ dependencies, stubs }, done);
+    });
+
+    it('should return no error', () => {
+      expect(error).to.be.null;
+    });
+
+    it('should symlink the missing dependencies', () => {
+      expect(stubs.ensureSymlinkSync.args[0][0]).to.deep.equal(
+        '/path/to/oc-running',
+        '/path/to/oc-running',
+        'dir'
+      );
+    });
+
+    it('should log progress', () => {
+      expect(logger.warn.args[0][0]).to.contain(
+        'Trying to link missing modules: lodash@1.2.3, underscore@latest'
+      );
+    });
+  });
+
+  describe('when there are missing dependencies and link fails', () => {
+    let dependencies, stubs;
+
+    beforeEach(done => {
+      stubs = {
+        getMissingDependencies: sinon.stub(),
+        ensureSymlinkSync: sinon.stub().throws(new Error('symlink error'))
+      };
+
+      stubs.getMissingDependencies
+        .onCall(0)
+        .returns(['lodash@1.2.3', 'underscore@latest']);
+
+      dependencies = { lodash: '1.2.3', underscore: '' };
+      initialise({ dependencies, stubs }, done);
+    });
+
+    it('should return the error', () => {
+      expect(error).to.equal('An error happened when linking the dependencies');
+    });
+
+    it('should log progress', () => {
+      expect(logger.warn.args[0][0]).to.contain(
+        'Trying to link missing modules: lodash@1.2.3, underscore@latest'
+      );
+      expect(logger.err.args[0][0]).to.equal(
+        'An error happened when linking the dependency lodash with error Error: symlink error'
+      );
+    });
+  });
+});