From 07d276977ae0588beb829865240df6ee4cf430d6 Mon Sep 17 00:00:00 2001 From: Pradnya Baviskar Date: Mon, 23 Mar 2015 13:49:11 +0530 Subject: [PATCH] Improve the resolution of relative paths - resolve module relative path for component - prioritize coffeescript over json --- lib/compiler.js | 123 +++++++++++------ test/compiler.test.js | 299 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 350 insertions(+), 72 deletions(-) diff --git a/lib/compiler.js b/lib/compiler.js index cfb6d54..5e16c9b 100644 --- a/lib/compiler.js +++ b/lib/compiler.js @@ -8,6 +8,8 @@ var debug = require('debug')('loopback:boot:compiler'); var Module = require('module'); var _ = require('lodash'); +var FILE_EXTENSION_JSON = '.json'; + /** * Gather all bootstrap-related configuration data and compile it into * a single object containing instruction for `boot.execute`. @@ -160,12 +162,11 @@ function findScripts(dir) { } var filepath = path.resolve(path.join(dir, filename)); - var ext = path.extname(filename); var stats = fs.statSync(filepath); // only require files supported by require.extensions (.txt .md etc.) if (stats.isFile()) { - if (ext in require.extensions) + if (isPreferredExtension(filename)) results.push(filepath); else debug('Skipping file %s - unknown extension', filepath); @@ -299,8 +300,8 @@ function findModelDefinitions(rootDir, sources) { return registry; } -function resolveAppPath(rootDir, relativePath) { - var resolvedPath = tryResolveAppPath(rootDir, relativePath); +function resolveAppPath(rootDir, relativePath, resolveOptions) { + var resolvedPath = tryResolveAppPath(rootDir, relativePath, resolveOptions); if (resolvedPath === undefined) { var err = new Error('Cannot resolve path "' + relativePath + '"'); err.code = 'PATH_NOT_FOUND'; @@ -309,12 +310,27 @@ function resolveAppPath(rootDir, relativePath) { return resolvedPath; } -function tryResolveAppPath(rootDir, relativePath) { - var fullPath = path.resolve(rootDir, relativePath); - if (fs.existsSync(fullPath)) +function tryResolveAppPath(rootDir, relativePath, resolveOptions) { + var fullPath; + var start = relativePath.substring(0, 2); + + /* In order to retain backward compatibility, while resolving + * component path, `resolveOptions` parameter is added where + * `resolveOptions.strict` = true, + * means retain backward compatibility when resolving the path and + * `resolveOptions.strict` = false, + * does not enforce any such restriction when resolving the path */ + resolveOptions = resolveOptions || { strict: false }; + + if (relativePath[0] === '/') { + fullPath = relativePath; + } else if (start === './' || start === '..' || !resolveOptions.strict) { + fullPath = path.resolve(rootDir, relativePath); + } + + if (fullPath && fs.existsSync(fullPath)) return fullPath; - var start = relativePath.substring(0, 2); if (start !== './' && start !== '..') { // Handle module-relative path, e.g. `loopback/common/models` @@ -339,6 +355,7 @@ function tryResolveAppPath(rootDir, relativePath) { return fs.existsSync(candidate); }) [0]; + if (fullPath) return fullPath; } else { @@ -356,27 +373,12 @@ function tryResolveAppPath(rootDir, relativePath) { function loadModelDefinition(rootDir, jsonFile, allFiles) { var definition = require(jsonFile); - var basename = path.basename(jsonFile, path.extname(jsonFile)); // find a matching file with a supported extension like `.js` or `.coffee` - var base; - var ext; - var validFileType; - var sourceFile = allFiles - .filter(function(f) { - ext = path.extname(f); - base = path.basename(f, ext); - validFileType = (ext !== '.node') && (ext !== '.json') && - ((typeof require.extensions[ext]) === 'function'); - return validFileType && (base === basename); - })[0]; + var sourceFile = fixFileExtension(jsonFile, allFiles, true); - try { - sourceFile = path.join(path.dirname(jsonFile), sourceFile); - sourceFile = require.resolve(sourceFile); - } catch (err) { - debug('Model source code not found: %s - %s', sourceFile, err.code || err); - sourceFile = undefined; + if (sourceFile === undefined) { + debug('Model source code not found: %s', sourceFile); } debug('Found model "%s" - %s %s', definition.name, @@ -456,7 +458,7 @@ function resolveMiddlewarePath(rootDir, middleware) { } if (!fragment) { - resolved.sourceFile = resolveAppPath(rootDir, middlewarePath); + resolved.sourceFile = resolveAppScriptPath(rootDir, middlewarePath); return resolved; } @@ -466,7 +468,7 @@ function resolveMiddlewarePath(rootDir, middleware) { // function var m = require(pathName); if (typeof m[fragment] === 'function') { - resolved.sourceFile = resolveAppPath(rootDir, middlewarePath); + resolved.sourceFile = resolveAppScriptPath(rootDir, middlewarePath); return resolved; } @@ -484,7 +486,7 @@ function resolveMiddlewarePath(rootDir, middleware) { for (var ix in candidates) { try { - resolved.sourceFile = resolveAppPath(rootDir, candidates[ix]); + resolved.sourceFile = resolveAppScriptPath(rootDir, candidates[ix]); delete resolved.fragment; return resolved; } @@ -513,16 +515,8 @@ function buildComponentInstructions(rootDir, componentConfig) { return Object.keys(componentConfig) .filter(function(name) { return !!componentConfig[name]; }) .map(function(name) { - var sourceFile; - if (name.indexOf('./') === 0 || name.indexOf('../') === 0) { - // Relative path - sourceFile = path.resolve(rootDir, name); - } else { - sourceFile = require.resolve(name); - } - return { - sourceFile: sourceFile, + sourceFile: resolveAppScriptPath(rootDir, name, {'strict': true}), config: componentConfig[name] }; }); @@ -538,3 +532,56 @@ function resolveRelativePaths(relativePaths, appRootDir) { } }); } + +function getExcludedExtensions() { + return { + '.json': '.json', + '.node': 'node' + }; +} + +function isPreferredExtension (filename) { + var includeExtensions = require.extensions; + + var ext = path.extname(filename); + return (ext in includeExtensions) && !(ext in getExcludedExtensions()); +} + +function fixFileExtension (filepath, files, onlyScriptsExportingFunction) { + var results = []; + var otherFile; + + /* Prefer coffee scripts over json */ + if (isPreferredExtension(filepath)) return filepath; + + var basename = path.basename(filepath, FILE_EXTENSION_JSON); + var sourceDir = path.dirname(filepath); + + files.forEach(function(f) { + otherFile = path.resolve(sourceDir, f); + + var stats = fs.statSync(otherFile); + if (stats.isFile()) { + var otherFileExtension = path.extname(f); + + if (!(otherFileExtension in getExcludedExtensions()) && + path.basename(f, otherFileExtension) == basename) { + if (!onlyScriptsExportingFunction) + results.push(otherFile); + else if (onlyScriptsExportingFunction && + (typeof require.extensions[otherFileExtension]) === 'function') { + results.push(otherFile); + } + } + } + }); + return (results.length > 0 ? results[0] : undefined); +} + +function resolveAppScriptPath (rootDir, relativePath, resolveOptions) { + var resolvedPath = resolveAppPath (rootDir, relativePath, resolveOptions); + var sourceDir = path.dirname(resolvedPath); + var files = tryReadDir(sourceDir); + var fixedFile = fixFileExtension (resolvedPath, files, false); + return (fixedFile === undefined ? resolvedPath : fixedFile); +} diff --git a/test/compiler.test.js b/test/compiler.test.js index 079aff1..cc3a5c2 100644 --- a/test/compiler.test.js +++ b/test/compiler.test.js @@ -5,6 +5,9 @@ var expect = require('chai').expect; var sandbox = require('./helpers/sandbox'); var appdir = require('./helpers/appdir'); +// add coffee-script to require.extensions +require('coffee-script/register'); + var SIMPLE_APP = path.join(__dirname, 'fixtures', 'simple-app'); describe('compiler', function() { @@ -394,6 +397,63 @@ describe('compiler', function() { expect(instructions.files.boot).to.eql([initJs]); }); + it('should resolve relative path in `bootDirs`', function() { + appdir.createConfigFilesSync(); + var initJs = appdir.writeFileSync('custom-boot/init.js', + 'module.exports = function(app) { app.fnCalled = true; };'); + var instructions = boot.compile({ + appRootDir: appdir.PATH, + bootDirs:['./custom-boot'] + }); + expect(instructions.files.boot).to.eql([initJs]); + }); + + it('should resolve non-relative path in `bootDirs`', function() { + appdir.createConfigFilesSync(); + var initJs = appdir.writeFileSync('custom-boot/init.js', ''); + var instructions = boot.compile({ + appRootDir: appdir.PATH, + bootDirs:['custom-boot'] + }); + expect(instructions.files.boot).to.eql([initJs]); + }); + + it('ignores index.js in `bootDirs`', function() { + appdir.createConfigFilesSync(); + appdir.writeFileSync('custom-boot/index.js', ''); + var instructions = boot.compile({ + appRootDir: appdir.PATH, + bootDirs:['./custom-boot'] + }); + expect(instructions.files.boot).to.have.length(0); + }); + + it('prefers coffeescript over json in `appRootDir/bootDir`', function() { + appdir.createConfigFilesSync(); + var coffee = appdir.writeFileSync('./custom-boot/init.coffee', ''); + appdir.writeFileSync('./custom-boot/init.json', {}); + + var instructions = boot.compile({ + appRootDir: appdir.PATH, + bootDirs: ['./custom-boot'] + }); + expect(instructions.files.boot).to.eql([coffee]); + }); + + it('prefers coffeescript over json in `bootDir` non-relative path', + function() { + appdir.createConfigFilesSync(); + var coffee = appdir.writeFileSync('custom-boot/init.coffee', + ''); + appdir.writeFileSync('custom-boot/init.json', ''); + + var instructions = boot.compile({ + appRootDir: appdir.PATH, + bootDirs: ['custom-boot'] + }); + expect(instructions.files.boot).to.eql([coffee]); + }); + it('supports `bootScripts` option', function() { appdir.createConfigFilesSync(); var initJs = appdir.writeFileSync('custom-boot/init.js', @@ -428,17 +488,6 @@ describe('compiler', function() { expect(instructions.files.boot).to.eql([initJs]); }); - it('should resolve relative path in `bootDirs`', function() { - appdir.createConfigFilesSync(); - var initJs = appdir.writeFileSync('custom-boot/init.js', - 'module.exports = function(app) { app.fnCalled = true; };'); - var instructions = boot.compile({ - appRootDir: appdir.PATH, - bootDirs:['./custom-boot'] - }); - expect(instructions.files.boot).to.eql([initJs]); - }); - it('should resolve non-relative path in `bootScripts`', function() { appdir.createConfigFilesSync(); var initJs = appdir.writeFileSync('custom-boot/init.js', ''); @@ -449,16 +498,6 @@ describe('compiler', function() { expect(instructions.files.boot).to.eql([initJs]); }); - it('should resolve non-relative path in `bootDirs`', function() { - appdir.createConfigFilesSync(); - var initJs = appdir.writeFileSync('custom-boot/init.js', ''); - var instructions = boot.compile({ - appRootDir: appdir.PATH, - bootDirs:['custom-boot'] - }); - expect(instructions.files.boot).to.eql([initJs]); - }); - it('resolves missing extensions in `bootScripts`', function() { appdir.createConfigFilesSync(); var initJs = appdir.writeFileSync('custom-boot/init.js', ''); @@ -481,16 +520,6 @@ describe('compiler', function() { expect(instructions.files.boot).to.eql([initJs]); }); - it('ignores index.js in `bootDirs`', function() { - appdir.createConfigFilesSync(); - appdir.writeFileSync('custom-boot/index.js', ''); - var instructions = boot.compile({ - appRootDir: appdir.PATH, - bootDirs:['./custom-boot'] - }); - expect(instructions.files.boot).to.have.length(0); - }); - it('resolves module relative path for `bootScripts`', function() { appdir.createConfigFilesSync(); var initJs = appdir.writeFileSync('node_modules/custom-boot/init.js', ''); @@ -564,9 +593,6 @@ describe('compiler', function() { }); it('loads coffeescript models from `./models`', function() { - // add coffee-script to require.extensions - require('coffee-script/register'); - appdir.createConfigFilesSync({}, {}, { Car: { dataSource: 'db' } }); @@ -662,6 +688,75 @@ describe('compiler', function() { }); }); + it('resolves relative path in `modelSources` option', function() { + appdir.createConfigFilesSync({}, {}, { + Car: { dataSource: 'db' } + }); + appdir.writeConfigFileSync('custom-models/car.json', { name: 'Car' }); + var appJS = appdir.writeFileSync('custom-models/car.js', ''); + + var instructions = boot.compile({ + appRootDir: appdir.PATH, + modelSources: ['./custom-models'] + }); + + expect(instructions.models).to.have.length(1); + expect(instructions.models[0].sourceFile).to.equal(appJS); + }); + + it('resolves module relative path in `modelSources` option', function() { + appdir.createConfigFilesSync({}, {}, { + Car: { dataSource: 'db' } + }); + appdir.writeConfigFileSync('node_modules/custom-models/car.json', + { name: 'Car' }); + var appJS = appdir.writeFileSync('node_modules/custom-models/car.js', ''); + + var instructions = boot.compile({ + appRootDir: appdir.PATH, + modelSources: ['custom-models'] + }); + + expect(instructions.models).to.have.length(1); + expect(instructions.models[0].sourceFile).to.equal(appJS); + }); + + it('resolves relative path in `sources` option in `model-config.json`', + function() { + appdir.createConfigFilesSync({}, {}, { + _meta: { + sources: ['./custom-models'] + }, + Car: { dataSource: 'db' } + }); + appdir.writeConfigFileSync('custom-models/car.json', { name: 'Car' }); + var appJS = appdir.writeFileSync('custom-models/car.js', ''); + + var instructions = boot.compile(appdir.PATH); + + expect(instructions.models).to.have.length(1); + expect(instructions.models[0].sourceFile).to.equal(appJS); + }); + + it('resolves module relative path in `sources` option in model-config.json', + function() { + appdir.createConfigFilesSync({}, {}, { + _meta: { + sources: ['custom-models'] + }, + Car: { dataSource: 'db' } + }); + appdir.writeConfigFileSync('node_modules/custom-models/car.json', + { name: 'Car' }); + + var appJS = appdir.writeFileSync('node_modules/custom-models/car.js', ''); + + var instructions = boot.compile(appdir.PATH); + + expect(instructions.models).to.have.length(1); + expect(instructions.models[0].sourceFile).to.equal(appJS); + }); + it('handles model definitions with no code', function() { appdir.createConfigFilesSync({}, {}, { Car: { dataSource: 'db' } @@ -1096,6 +1191,73 @@ describe('compiler', function() { 'sourceFile', moduleJS); }); + it('loads middleware from coffeescript in appRootdir', function() { + var coffee = appdir.writeFileSync('my-middleware.coffee', ''); + appdir.writeConfigFileSync('middleware.json', { + 'routes': { + './my-middleware': {} + } + }); + + var instructions = boot.compile(appdir.PATH); + + expect(instructions.middleware.middleware[0]).have.property( + 'sourceFile', coffee); + }); + + it('loads coffeescript from middleware under node_modules', + function() { + var file = appdir.writeFileSync('node_modules/my-middleware/index.coffee', + ''); + appdir.writeFileSync('node_modules/my-middleware/index.json', ''); + appdir.writeConfigFileSync('middleware.json', { + 'routes': { + 'my-middleware': {} + } + }); + + var instructions = boot.compile(appdir.PATH); + + expect(instructions.middleware.middleware).to.have.length(1); + expect(instructions.middleware.middleware[0]).have.property( + 'sourceFile', file); + }); + + it('prefers coffeescript over json for relative middleware path', + function() { + var coffee = appdir.writeFileSync('my-middleware.coffee', ''); + appdir.writeFileSync('my-middleware.json', ''); + appdir.writeConfigFileSync('middleware.json', { + 'routes': { + './my-middleware': {} + } + }); + + var instructions = boot.compile(appdir.PATH); + + expect(instructions.middleware.middleware).to.have.length(1); + expect(instructions.middleware.middleware[0]).have.property( + 'sourceFile', coffee); + }); + + it('prefers coffeescript over json for module relative middleware path', + function() { + var coffee = appdir.writeFileSync('node_modules/my-middleware.coffee', + ''); + appdir.writeFileSync('node_modules/my-middleware.json', ''); + appdir.writeConfigFileSync('middleware.json', { + 'routes': { + 'my-middleware': {} + } + }); + + var instructions = boot.compile(appdir.PATH); + + expect(instructions.middleware.middleware).to.have.length(1); + expect(instructions.middleware.middleware[0]).have.property( + 'sourceFile', coffee); + }); + describe('config with relative paths in params', function() { var RELATIVE_PATH_PARAMS = [ '$!./here', @@ -1195,6 +1357,75 @@ describe('compiler', function() { } }); }); + + it('loads component relative to appRootDir', function() { + appdir.writeConfigFileSync('./component-config.json', { + './index': { } + }); + var appJS = appdir.writeConfigFileSync('index.js', ''); + + var instructions = boot.compile(appdir.PATH); + expect(instructions.components[0]).have.property( + 'sourceFile', appJS + ); + }); + + it('loads component relative to node modules', function() { + appdir.writeConfigFileSync('component-config.json', { + 'mycomponent': { } + }); + var js = appdir.writeConfigFileSync('node_modules/mycomponent/index.js', + ''); + + var instructions = boot.compile(appdir.PATH); + expect(instructions.components[0]).have.property( + 'sourceFile', js + ); + }); + + it('retains backward compatibility for non-relative path in `appRootDir`', + function() { + appdir.writeConfigFileSync('component-config.json', { + 'my-component/component.js': { } + }); + appdir.writeConfigFileSync('./my-component/component.js', ''); + + expect(function() { boot.compile(appdir.PATH); }) + .to.throw('Cannot resolve path \"my-component/component.js\"'); + }); + + it('prefers coffeescript over json for relative path component', + function() { + appdir.writeConfigFileSync('component-config.json', { + './component': { } + }); + + var coffee = appdir.writeFileSync('component.coffee', ''); + appdir.writeFileSync('component.json', ''); + + var instructions = boot.compile(appdir.PATH); + + expect(instructions.components).to.have.length(1); + expect(instructions.components[0]).have.property( + 'sourceFile', coffee); + }); + + it('prefers coffeescript over json for module relative component path', + function() { + appdir.writeConfigFileSync('component-config.json', { + 'component': { } + }); + + var coffee = appdir.writeFileSync('node_modules/component.coffee', ''); + appdir.writeFileSync('node_modules/component.json', ''); + + var instructions = boot.compile(appdir.PATH); + + expect(instructions.components).to.have.length(1); + expect(instructions.components[0]).have.property( + 'sourceFile', coffee); + }); + }); });