Skip to content
This repository was archived by the owner on Dec 4, 2022. It is now read-only.

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

Merged
merged 4 commits into from
Jun 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
4 changes: 1 addition & 3 deletions src/dependency-builder/dependency-tree/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
49 changes: 49 additions & 0 deletions src/dependency-builder/dependency-tree/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`);
Expand Down Expand Up @@ -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');
});
});
});
36 changes: 16 additions & 20 deletions src/dependency-builder/filing-cabinet/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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
Expand All @@ -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');

Expand All @@ -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 : '';
Expand Down Expand Up @@ -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');

Expand All @@ -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);
}
Expand All @@ -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}`);
}
Expand Down
33 changes: 17 additions & 16 deletions src/dependency-builder/lookups/vue-lookup/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];
};
2 changes: 1 addition & 1 deletion src/utils/is-relative-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 === '..';
}