fix dependency resolution of . and .. to not be identified as custom-resolved used (#105)

* fix teambit/bit#1734, recognize "." and ".." partials as relative paths

*  avoid assuming that custom-resolve was used only based on the relative-path and the existence of the config. instead, explicitly mark custom-resolved as such.

* bump version

Author: davidfirst

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [unreleased]
 
+## [2.0.12-angular.3] - 2019-06-14
+
+- fix dependency resolution of `.` and `..` to not be identified as custom-resolved used.
+
 ## [2.0.12-angular.2] - 2019-06-10
 
 - add rxjs package

package.json

@@ -1,6 +1,6 @@
 {
   "name": "bit-javascript",
-  "version": "2.0.12-angular.2",
+  "version": "2.0.12-angular.3",
   "scripts": {
     "flow": "flow; test $? -eq 0 -o $? -eq 2",
     "lint": "eslint src && flow check || true",

src/dependency-builder/dependency-tree/index.js

@@ -139,9 +139,7 @@ module.exports._getDependencies = function (config) {
     if (!isDependenciesArray && dependenciesRaw[dependency].importSpecifiers) {
       pathMap.importSpecifiers = dependenciesRaw[dependency].importSpecifiers;
     }
-    if (!isRelativeImport(dependency) && config.resolveConfig) {
-      // is includes also packages, which actually don't use customResolve, however, they will be
-      // filtered out later.
+    if (cabinetParams.wasCustomResolveUsed) {
       pathMap.isCustomResolveUsed = true;
     }
 

src/dependency-builder/dependency-tree/index.spec.js

@@ -16,6 +16,7 @@ require('../../utils');
 require('../../utils/is-relative-import');
 require('../../dependency-builder/detectives/detective-css-and-preprocessors');
 require('../../dependency-builder/detectives/detective-typescript');
+require('../../dependency-builder/detectives/parser-helper');
 
 const dependencyTree = rewire('./');
 const fixtures = path.resolve(`${__dirname}/../../../fixtures/dependency-tree`);
@@ -918,4 +919,52 @@ describe('dependencyTree', function () {
       expect(visited[filename].missing).to.be.undefined;
     });
   });
+  describe('resolve config when the dependency is "."', () => {
+    it('should not set the dependency with isCustomResolveUsed=true', () => {
+      mockfs({
+        [`${__dirname}/src`]: {
+          'foo.js': "require('.');",
+          'index.js': 'module.exports = {}'
+        }
+      });
+      const directory = path.normalize(`${__dirname}/src`);
+      const filename = path.normalize(`${directory}/foo.js`);
+      const config = {
+        filename,
+        directory,
+        pathMap: [],
+        resolveConfig: { aliases: { something: 'anything' } }
+      };
+      dependencyTree(config);
+      const pathMapRecord = config.pathMap.find(f => f.file === filename);
+      expect(pathMapRecord.dependencies).to.have.lengthOf(1);
+      const dependency = pathMapRecord.dependencies[0];
+      expect(dependency).to.not.have.property('isCustomResolveUsed');
+    });
+  });
+  describe('resolve config when the dependency is ".."', () => {
+    it('should not set the dependency with isCustomResolveUsed=true', () => {
+      mockfs({
+        [`${__dirname}/src`]: {
+          'index.js': 'module.exports = {}',
+          bar: {
+            'foo.js': "require('..');"
+          }
+        }
+      });
+      const directory = path.normalize(`${__dirname}/src`);
+      const filename = path.normalize(`${directory}/bar/foo.js`);
+      const config = {
+        filename,
+        directory,
+        pathMap: [],
+        resolveConfig: { aliases: { something: 'anything' } }
+      };
+      dependencyTree(config);
+      const pathMapRecord = config.pathMap.find(f => f.file === filename);
+      expect(pathMapRecord.dependencies).to.have.lengthOf(1);
+      const dependency = pathMapRecord.dependencies[0];
+      expect(dependency).to.not.have.property('isCustomResolveUsed');
+    });
+  });
 });

src/dependency-builder/filing-cabinet/index.js

@@ -52,7 +52,8 @@ type Options = {
   isScript?: boolean, // relevant for Vue files
   ast?: string,
   ext?: string,
-  content?: string
+  content?: string,
+  wasCustomResolveUsed?: boolean
 };
 
 module.exports = function cabinet(options: Options) {
@@ -163,18 +164,7 @@ module.exports._getJSType = function (options) {
  * @return {String}
  */
 function jsLookup(options: Options) {
-  const {
-    configPath,
-    partial,
-    directory,
-    config,
-    webpackConfig,
-    filename,
-    ast,
-    isScript,
-    content,
-    resolveConfig
-  } = options;
+  const { configPath, partial, directory, config, webpackConfig, filename, ast, isScript, content } = options;
   const type = module.exports._getJSType({
     config,
     webpackConfig,
@@ -204,7 +194,7 @@ function jsLookup(options: Options) {
     case 'es6':
     default:
       debug(`using commonjs resolver ${type}`);
-      return commonJSLookup(partial, filename, directory, resolveConfig);
+      return commonJSLookup(options);
   }
 }
 
@@ -225,7 +215,10 @@ function cssPreprocessorLookup(options: Options) {
   const { filename, partial, directory, resolveConfig } = options;
   if (resolveConfig && !isRelativeImport(partial)) {
     const result = resolveNonRelativePath(partial, filename, directory, resolveConfig);
-    if (result) return result;
+    if (result) {
+      options.wasCustomResolveUsed = true;
+      return result;
+    }
   }
   if (partial.startsWith('~') && !partial.startsWith('~/')) {
     // webpack syntax for resolving a module from node_modules
@@ -239,10 +232,10 @@ function cssPreprocessorLookup(options: Options) {
 }
 
 function tsLookup(options: Options) {
-  const { partial, filename, directory, resolveConfig } = options;
+  const { partial, filename } = options;
   if (partial[0] !== '.') {
     // when a path is not relative, use the standard commonJS lookup
-    return commonJSLookup(partial, filename, directory, resolveConfig);
+    return commonJSLookup(options);
   }
   debug('performing a typescript lookup');
 
@@ -262,7 +255,7 @@ function tsLookup(options: Options) {
   if (!resolvedModule) {
     // ts.resolveModuleName doesn't always work, fallback to commonJSLookup
     debug('failed resolving with tsLookup, trying commonJSLookup');
-    return commonJSLookup(partial, filename, directory, resolveConfig);
+    return commonJSLookup(options);
   }
   debug('ts resolved module: ', resolvedModule);
   const result = resolvedModule ? resolvedModule.resolvedFileName : '';
@@ -291,8 +284,9 @@ function resolveNonRelativePath(partial, filename, directory, resolveConfig) {
  * @param  {String} directory
  * @return {String}
  */
-function commonJSLookup(partial, filename, directory, resolveConfig) {
-  directory = path.dirname(filename); // node_modules should be propagated from the file location backwards
+function commonJSLookup(options: Options) {
+  const { filename, resolveConfig } = options;
+  const directory = path.dirname(filename); // node_modules should be propagated from the file location backwards
   // Need to resolve partials within the directory of the module, not filing-cabinet
   const moduleLookupDir = path.join(directory, 'node_modules');
 
@@ -302,6 +296,7 @@ function commonJSLookup(partial, filename, directory, resolveConfig) {
 
   // Make sure the partial is being resolved to the filename's context
   // 3rd party modules will not be relative
+  let partial = options.partial;
   if (partial[0] === '.') {
     partial = path.resolve(path.dirname(filename), partial);
   }
@@ -319,6 +314,7 @@ function commonJSLookup(partial, filename, directory, resolveConfig) {
     if (!isRelativeImport(partial) && resolveConfig) {
       debug(`trying to resolve using resolveConfig ${JSON.stringify(resolveConfig)}`);
       result = resolveNonRelativePath(partial, filename, directory, resolveConfig);
+      if (result) options.wasCustomResolveUsed = true;
     } else {
       debug(`could not resolve ${partial}`);
     }

src/dependency-builder/lookups/vue-lookup/index.js

@@ -17,25 +17,26 @@ module.exports = function (options) {
   const { script, styles } = compiler.parseComponent(fileContent.toString(), { pad: 'line' });
   if (isScript) {
     const scriptExt = script.lang ? languageMap[script.lang] || script.lang : DEFAULT_SCRIPT_LANG;
-    return cabinet({
-      partial,
-      filename,
-      directory: path.dirname(filename),
-      content: script.content,
-      resolveConfig,
-      ext: `.${scriptExt}` || path.extname(partial)
-    });
+    return cabinet(
+      Object.assign(options, {
+        directory: path.dirname(filename),
+        content: script.content,
+        ast: null,
+        ext: `.${scriptExt}` || path.extname(partial)
+      })
+    );
   }
   const stylesResult = styles.map((style) => {
     const styleExt = style.lang ? languageMap[style.lang] || style.lang : DEFAULT_STYLE_LANG;
-    return cabinet({
-      partial,
-      filename: `${path.join(path.dirname(filename), path.parse(filename).name)}.${styleExt}`,
-      directory: path.dirname(filename),
-      content: style.content,
-      resolveConfig,
-      ext: `.${styleExt}`
-    });
+    return cabinet(
+      Object.assign(options, {
+        filename: `${path.join(path.dirname(filename), path.parse(filename).name)}.${styleExt}`,
+        directory: path.dirname(filename),
+        content: style.content,
+        ast: null,
+        ext: `.${styleExt}`
+      })
+    );
   });
   return stylesResult[0];
 };

src/utils/is-relative-import.js

@@ -11,5 +11,5 @@
  * End quote.
  */
 export default function isRelativeImport(str: string): boolean {
-  return str.startsWith('./') || str.startsWith('../') || str.startsWith('/');
+  return str.startsWith('./') || str.startsWith('../') || str.startsWith('/') || str === '.' || str === '..';
 }