From c937f79a1cae9d9b50c09e1a15179f5c202b4471 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Ruci=C5=84ski?= Date: Tue, 18 Apr 2017 05:09:09 +0200 Subject: [PATCH] Feat: warn when the package from resolved alias is not available (#160) Breaking change: The "npm:" prefix has been removed. --- package.json | 3 +- src/getRealPath.js | 66 ++++++--------- src/log.js | 7 ++ src/normalizeOptions.js | 43 +++++----- src/utils.js | 9 ++ test/import.test.js | 10 +++ test/index.test.js | 177 ++++++++++++++++++++++++++-------------- 7 files changed, 193 insertions(+), 122 deletions(-) create mode 100644 src/log.js diff --git a/package.json b/package.json index 0411459..3dda89f 100644 --- a/package.json +++ b/package.json @@ -62,7 +62,8 @@ "jest": { "testRegex": "/test/.*\\.test\\.js$", "collectCoverageFrom": [ - "src/**/*.js" + "src/**/*.js", + "!src/log.js" ] }, "greenkeeper": { diff --git a/src/getRealPath.js b/src/getRealPath.js index 6d14050..6aef66e 100644 --- a/src/getRealPath.js +++ b/src/getRealPath.js @@ -1,8 +1,8 @@ import path from 'path'; -import resolve from 'resolve'; +import { warn } from './log'; import mapToRelative from './mapToRelative'; -import { toLocalPath, toPosixPath, replaceExtension } from './utils'; +import { nodeResolvePath, replaceExtension, toLocalPath, toPosixPath } from './utils'; function findPathInRoots(sourcePath, { extensions, root }) { @@ -10,16 +10,8 @@ function findPathInRoots(sourcePath, { extensions, root }) { let resolvedSourceFile; root.some((basedir) => { - try { - // Check if the file exists (will throw if not) - resolvedSourceFile = resolve.sync(`./${sourcePath}`, { - basedir, - extensions, - }); - return true; - } catch (e) { - return false; - } + resolvedSourceFile = nodeResolvePath(`./${sourcePath}`, basedir, extensions); + return resolvedSourceFile !== null; }); return resolvedSourceFile; @@ -43,40 +35,17 @@ function getRealPathFromRootConfig(sourcePath, currentFile, opts) { ))); } -function getRealPathFromAliasConfig(sourcePath, currentFile, { alias, cwd }) { - const moduleSplit = sourcePath.split('/'); - let aliasPath; - - while (moduleSplit.length) { - const m = moduleSplit.join('/'); - if ({}.hasOwnProperty.call(alias, m)) { - aliasPath = alias[m]; - break; - } - moduleSplit.pop(); - } - - // no alias mapping found - if (!aliasPath) { - return null; - } - - // remove legacy "npm:" prefix for npm packages - aliasPath = aliasPath.replace(/^(npm:)/, ''); - const newPath = sourcePath.replace(moduleSplit.join('/'), aliasPath); - - // alias to npm module don't need relative mapping - if (aliasPath[0] !== '.') { - return newPath; +function checkIfPackageExists(modulePath, currentFile, extensions) { + const resolvedPath = nodeResolvePath(modulePath, currentFile, extensions); + if (resolvedPath === null) { + warn(`Could not resolve "${modulePath}" in file ${currentFile}.`); } - - return toLocalPath(toPosixPath(mapToRelative(cwd, currentFile, newPath))); } -function getRealPathFromRegExpConfig(sourcePath, currentFile, { regExps }) { +function getRealPathFromAliasConfig(sourcePath, currentFile, opts) { let aliasedSourceFile; - regExps.find(([regExp, substitute]) => { + opts.alias.find(([regExp, substitute]) => { const execResult = regExp.exec(sourcePath); if (execResult === null) { @@ -87,13 +56,26 @@ function getRealPathFromRegExpConfig(sourcePath, currentFile, { regExps }) { return true; }); + if (!aliasedSourceFile) { + return null; + } + + if (aliasedSourceFile[0] === '.') { + return toLocalPath(toPosixPath( + mapToRelative(opts.cwd, currentFile, aliasedSourceFile)), + ); + } + + if (process.env.NODE_ENV !== 'production') { + checkIfPackageExists(aliasedSourceFile, currentFile, opts.extensions); + } + return aliasedSourceFile; } const resolvers = [ getRealPathFromRootConfig, getRealPathFromAliasConfig, - getRealPathFromRegExpConfig, ]; export default function getRealPath(sourcePath, { file, opts }) { diff --git a/src/log.js b/src/log.js new file mode 100644 index 0000000..9c45b16 --- /dev/null +++ b/src/log.js @@ -0,0 +1,7 @@ +// This module exists only for abstracting logging away and making testing easier + +// eslint-disable-next-line import/prefer-default-export +export function warn(...args) { + // eslint-disable-next-line no-console + console.warn(...args); +} diff --git a/src/normalizeOptions.js b/src/normalizeOptions.js index 5bd073f..7698d75 100644 --- a/src/normalizeOptions.js +++ b/src/normalizeOptions.js @@ -60,29 +60,32 @@ function normalizeRoot(opts) { } } -function normalizeAlias(opts) { - opts.regExps = []; - - if (opts.alias) { - Object.keys(opts.alias) - .filter(isRegExp) - .forEach((key) => { - const parts = opts.alias[key].split('\\\\'); - - function substitute(execResult) { - return parts - .map(part => - part.replace(/\\\d+/g, number => execResult[number.slice(1)] || ''), - ) - .join('\\'); - } +function getAliasPair(key, value) { + const parts = value.split('\\\\'); + + function substitute(execResult) { + return parts + .map(part => + part.replace(/\\\d+/g, number => execResult[number.slice(1)] || ''), + ) + .join('\\'); + } - opts.regExps.push([new RegExp(key), substitute]); + return [new RegExp(key), substitute]; +} - delete opts.alias[key]; - }); +function normalizeAlias(opts) { + if (opts.alias) { + const { alias } = opts; + const aliasKeys = Object.keys(alias); + + opts.alias = aliasKeys.map(key => ( + isRegExp(key) ? + getAliasPair(key, alias[key]) : + getAliasPair(`^${key}((?:/|).*)`, `${alias[key]}\\1`) + )); } else { - opts.alias = {}; + opts.alias = []; } } diff --git a/src/utils.js b/src/utils.js index e849491..6c2a4b4 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1,8 +1,17 @@ import path from 'path'; +import resolve from 'resolve'; import getRealPath from './getRealPath'; +export function nodeResolvePath(modulePath, basedir, extensions) { + try { + return resolve.sync(modulePath, { basedir, extensions }); + } catch (e) { + return null; + } +} + export function toPosixPath(modulePath) { return modulePath.replace(/\\/g, '/'); } diff --git a/test/import.test.js b/test/import.test.js index ebbaddf..6836b92 100644 --- a/test/import.test.js +++ b/test/import.test.js @@ -84,6 +84,7 @@ describe('import and export statement', () => { root: './test/testproject/src', alias: { test: './test/testproject/test', + 'babel-core': 'babel-core/lib', }, }], ], @@ -121,6 +122,15 @@ describe('import and export statement', () => { ); }); + describe('should only apply the alias once', () => { + // If this test breaks, consider selecting another package used by the plugin + testImports( + 'babel-core/store', + 'babel-core/lib/store', + transformerOpts, + ); + }); + describe('should ignore the call if a non-import statement is used', () => { const code = stripIndent` function test() { diff --git a/test/index.test.js b/test/index.test.js index b91148f..1d7386a 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -209,6 +209,28 @@ describe('module-resolver', () => { ); }); }); + + describe('root and alias', () => { + const rootTransformerOpts = { + babelrc: false, + plugins: [ + [plugin, { + root: './test/testproject/src', + alias: { + constants: 'constants/actions', + }, + }], + ], + }; + + it('should resolve the path using root first and alias otherwise', () => { + testWithImport( + 'constants', + './test/testproject/src/constants', + rootTransformerOpts, + ); + }); + }); }); describe('alias', () => { @@ -221,16 +243,14 @@ describe('module-resolver', () => { components: './test/testproject/src/components', '~': './test/testproject/src', 'awesome/components': './test/testproject/src/components', - abstract: 'npm:concrete', - underscore: 'lodash', - prefix: 'prefix/lib', - '^@namespace/foo-(.+)': 'packages/\\1', - 'styles/.+\\.(css|less|scss)$': 'style-proxy.\\1', - '^single-backslash': 'pas\\\\sed', - '^non-existing-match': 'pas\\42sed', - '^regexp-priority': 'miss', - 'regexp-priority$': 'miss', - 'regexp-priority': 'hit', + 'babel-kernel': 'babel-core', + '^@namespace/foo-(.+)': './packages/\\1', + 'styles/.+\\.(css|less|scss)$': './style-proxy.\\1', + '^single-backslash': './pas\\\\sed', + '^non-existing-match': './pas\\42sed', + '^regexp-priority': './hit', + 'regexp-priority$': './miss', + 'regexp-priority': './miss', }, }], ], @@ -306,18 +326,11 @@ describe('module-resolver', () => { }); }); - it('(legacy) should support aliasing a node module with "npm:"', () => { + it('should support aliasing a node module', () => { + // If this test breaks, consider selecting another package used by the plugin testWithImport( - 'abstract/thing', - 'concrete/thing', - aliasTransformerOpts, - ); - }); - - it('should support aliasing a node modules', () => { - testWithImport( - 'underscore/map', - 'lodash/map', + 'babel-kernel/register', + 'babel-core/register', aliasTransformerOpts, ); }); @@ -326,7 +339,7 @@ describe('module-resolver', () => { it('should support replacing parts of a path', () => { testWithImport( '@namespace/foo-bar', - 'packages/bar', + './packages/bar', aliasTransformerOpts, ); }); @@ -334,7 +347,7 @@ describe('module-resolver', () => { it('should support replacing parts of a complex path', () => { testWithImport( '@namespace/foo-bar/component.js', - 'packages/bar/component.js', + './packages/bar/component.js', aliasTransformerOpts, ); }); @@ -344,7 +357,7 @@ describe('module-resolver', () => { it(`should handle the alias with the ${extension} extension`, () => { testWithImport( `styles/style.${extension}`, - `style-proxy.${extension}`, + `./style-proxy.${extension}`, aliasTransformerOpts, ); }); @@ -359,19 +372,19 @@ describe('module-resolver', () => { ); }); - it('should transform a double backslash into a single one', () => { + it('should unescape a double backslash into a single one', () => { testWithImport( 'single-backslash', // This is a string literal, so in the code it will actually be "pas\\sed" - 'pas\\\\sed', + './pas/sed', aliasTransformerOpts, ); }); - it('should replece missing matches with an empty string', () => { + it('should replace missing matches with an empty string', () => { testWithImport( 'non-existing-match', - 'passed', + './passed', aliasTransformerOpts, ); }); @@ -379,7 +392,7 @@ describe('module-resolver', () => { it('should have higher priority than a simple alias', () => { testWithImport( 'regexp-priority', - 'hit', + './hit', aliasTransformerOpts, ); }); @@ -391,7 +404,7 @@ describe('module-resolver', () => { [plugin, { root: '.' }], [plugin, { alias: { - '^@namespace/foo-(.+)': 'packages/\\1', + '^@namespace/foo-(.+)': './packages/\\1', }, }], ], @@ -400,11 +413,75 @@ describe('module-resolver', () => { it('should support replacing parts of a path', () => { testWithImport( '@namespace/foo-bar', - 'packages/bar', + './packages/bar', doubleAliasTransformerOpts, ); }); }); + + describe('missing packages warning', () => { + const mockWarn = jest.fn(); + jest.mock('../src/log', () => ({ + warn: mockWarn, + })); + jest.resetModules(); + const pluginWithMock = require.requireActual('../src').default; + const fileName = path.resolve('unknown'); + + const missingAliasTransformerOpts = { + plugins: [ + [pluginWithMock, { + alias: { + legacy: 'npm:legacy', + 'non-existing': 'this-package-does-not-exist', + }, + }], + ], + }; + + beforeEach(() => { + mockWarn.mockClear(); + process.env.NODE_ENV = 'development'; + }); + + it('should print a warning for a legacy alias', () => { + testWithImport( + 'legacy/lib', + 'npm:legacy/lib', + missingAliasTransformerOpts, + ); + + expect(mockWarn.mock.calls.length).toBe(1); + expect(mockWarn).toBeCalledWith(`Could not resolve "npm:legacy/lib" in file ${fileName}.`); + }); + + it('should print a warning for an unresolved package', () => { + testWithImport( + 'non-existing/lib', + 'this-package-does-not-exist/lib', + missingAliasTransformerOpts, + ); + + expect(mockWarn.mock.calls.length).toBe(1); + expect(mockWarn).toBeCalledWith(`Could not resolve "this-package-does-not-exist/lib" in file ${fileName}.`); + }); + + describe('production environment', () => { + beforeEach(() => { + process.env.NODE_ENV = 'production'; + }); + + it('should print a warning for an unresolved package', () => { + testWithImport( + 'non-existing/lib', + 'this-package-does-not-exist/lib', + missingAliasTransformerOpts, + ); + + expect(mockWarn.mock.calls.length).toBe(0); + }); + }); + }); }); describe('with custom cwd', () => { @@ -416,10 +493,8 @@ describe('module-resolver', () => { root: './testproject/src', cwd: path.resolve('test'), alias: { - constantsRelative: './constants', - constantsNonRelative: 'constants', - '^constantsRegExpRelative(.*)': './constants\\1', - '^constantsNonRelative(.*)': 'constants\\1', + constantsAlias: './constants', + '^constantsRegExp(.*)': './constants\\1', }, }], ], @@ -433,34 +508,18 @@ describe('module-resolver', () => { ); }); - it('should alias the relative path while ignoring cwd and root', () => { + it('should alias the relative path while honoring cwd', () => { testWithImport( - 'constantsRelative/actions', + 'constantsAlias/actions', './test/constants/actions', transformerOpts, ); }); - it('should alias the non-relative path while ignoring cwd and root', () => { + it('should alias the relative path while honoring cwd', () => { testWithImport( - 'constantsNonRelative/actions', - 'constants/actions', - transformerOpts, - ); - }); - - it('should alias the relative path while ignoring cwd and root', () => { - testWithImport( - 'constantsRegExpRelative/actions', - './constants/actions', - transformerOpts, - ); - }); - - it('should alias the non-relative path while ignoring cwd and root', () => { - testWithImport( - 'constantsNonRelative/actions', - 'constants/actions', + 'constantsRegExp/actions', + './test/constants/actions', transformerOpts, ); }); @@ -514,7 +573,7 @@ describe('module-resolver', () => { [plugin, { root: './src', alias: { - actions: 'constants/actions', + test: './test', }, cwd: 'babelrc', }], @@ -532,8 +591,8 @@ describe('module-resolver', () => { it('should alias the sub file path', () => { testWithImport( - 'actions', - 'constants/actions', + 'test/tools', + '../test/tools', transformerOpts, ); });