From 53f51820f573ba4acbd09bc2fde94b98ea93d03a Mon Sep 17 00:00:00 2001 From: Pradnya Baviskar Date: Thu, 30 Apr 2015 11:23:44 +0530 Subject: [PATCH] support 'mixinsources' option - By default looks for mixinsources in directory - Loads only mixins used through models --- lib/compiler.js | 76 +++++++++++++++-- test/browser.test.js | 3 +- test/compiler.test.js | 187 ++++++++++++++++++++++++++++++++++++------ test/executor.test.js | 25 ++++-- 4 files changed, 248 insertions(+), 43 deletions(-) diff --git a/lib/compiler.js b/lib/compiler.js index 70459ab..3c89be0 100644 --- a/lib/compiler.js +++ b/lib/compiler.js @@ -85,8 +85,9 @@ module.exports = function compile(options) { modelsRootDir, modelsConfig, modelSources); var mixinDirs = options.mixinDirs || []; + var mixinSources = options.mixinSources || modelsMeta.mixins || ['./mixins']; var mixinInstructions = buildAllMixinInstructions( - appRootDir, mixinDirs, options); + appRootDir, mixinDirs, mixinSources, options, modelInstructions); // When executor passes the instruction to loopback methods, // loopback modifies the data. Since we are loading the data using `require`, @@ -606,12 +607,48 @@ function resolveAppScriptPath(rootDir, relativePath, resolveOptions) { return (fixedFile === undefined ? resolvedPath : fixedFile); } -function buildAllMixinInstructions(appRootDir, mixinDirs, options) { +function buildAllMixinInstructions(appRootDir, mixinDirs, mixinSources, options, + modelInstructions) { var extensions = _.without(_.keys(require.extensions), _.keys(getExcludedExtensions())); - var files = options.mixins || []; - mixinDirs.forEach(function(dir) { + // load mixins from `options.mixins` + var sourceFiles = options.mixins || []; + var instructionsFromMixins = loadMixins(sourceFiles, options); + + // load mixins from `options.mixinDirs` + sourceFiles = findMixinDefinitions(appRootDir, mixinDirs, extensions); + if (sourceFiles === undefined) return; + var instructionsFromMixinDirs = loadMixins(sourceFiles, options); + + /* If `mixinDirs` and `mixinSources` have any directories in common, + * then remove the common directories from `mixinSources` */ + mixinSources = _.difference(mixinSources, mixinDirs); + + // load mixins from `options.mixinSources` + sourceFiles = findMixinDefinitions(appRootDir, mixinSources, extensions); + if (sourceFiles === undefined) return; + var instructionsFromMixinSources = loadMixins(sourceFiles, options); + + // Fetch unique list of mixin names, used in models + var modelMixins = fetchMixinNamesUsedInModelInstructions(modelInstructions); + modelMixins = _.unique(modelMixins); + + // Filter-in only mixins, that are used in models + instructionsFromMixinSources = filterMixinInstructionsUsingWhitelist( + instructionsFromMixinSources, modelMixins); + + var mixins = _.assign( + instructionsFromMixins, + instructionsFromMixinDirs, + instructionsFromMixinSources); + + return _.values(mixins); +} + +function findMixinDefinitions(appRootDir, sourceDirs, extensions) { + var files = []; + sourceDirs.forEach(function(dir) { dir = tryResolveAppPath(appRootDir, dir); if (!dir) { debug('Skipping unknown module source dir %j', dir); @@ -619,8 +656,12 @@ function buildAllMixinInstructions(appRootDir, mixinDirs, options) { } files = files.concat(findScripts(dir, extensions)); }); + return files; +} - var mixins = files.map(function(filepath) { +function loadMixins(sourceFiles, options) { + var mixinInstructions = {}; + sourceFiles.forEach(function(filepath) { var dir = path.dirname(filepath); var ext = path.extname(filepath); var name = path.basename(filepath, ext); @@ -634,10 +675,31 @@ function buildAllMixinInstructions(appRootDir, mixinDirs, options) { _.extend(meta, require(metafile)); } meta.sourceFile = filepath; - return meta; + mixinInstructions[meta.name] = meta; }); - return mixins; + return mixinInstructions; +} + +function fetchMixinNamesUsedInModelInstructions(modelInstructions) { + return _.flatten(modelInstructions + .map(function(model) { + return model.definition && model.definition.mixins ? + Object.keys(model.definition.mixins) : []; + })); +} + +function filterMixinInstructionsUsingWhitelist(instructions, includeMixins) { + var instructionKeys = Object.keys(instructions); + includeMixins = _.intersection(instructionKeys, includeMixins); + + var filteredInstructions = {}; + instructionKeys.forEach(function(mixinName) { + if (includeMixins.indexOf(mixinName) !== -1) { + filteredInstructions[mixinName] = instructions[mixinName]; + } + }); + return filteredInstructions; } function normalizeMixinName(str, options) { diff --git a/test/browser.test.js b/test/browser.test.js index 8a92f5a..44e0b7a 100644 --- a/test/browser.test.js +++ b/test/browser.test.js @@ -64,8 +64,7 @@ describe('browser support', function() { it('loads mixins', function(done) { var appDir = path.resolve(__dirname, './fixtures/browser-app'); var options = { - appRootDir: appDir, - mixinDirs: ['./mixins'] + appRootDir: appDir }; browserifyTestApp(options, function(err, bundlePath) { diff --git a/test/compiler.test.js b/test/compiler.test.js index 9ed42e3..1b47db1 100644 --- a/test/compiler.test.js +++ b/test/compiler.test.js @@ -1016,41 +1016,175 @@ describe('compiler', function() { }); describe('for mixins', function() { - function verifyMixinIsFoundViaMixinDirs(sourceFile, mixinDirs) { - var appJS = appdir.writeFileSync(sourceFile, ''); + describe(' - mixinDirs', function() { + function verifyMixinIsFoundViaMixinDirs(sourceFile, mixinDirs) { + var appJS = appdir.writeFileSync(sourceFile, ''); - var instructions = boot.compile({ - appRootDir: appdir.PATH, - mixinDirs: mixinDirs + var instructions = boot.compile({ + appRootDir: appdir.PATH, + mixinDirs: mixinDirs + }); + + expect(instructions.mixins[0].sourceFile).to.eql(appJS); + } + + it('supports `mixinDirs` option', function() { + verifyMixinIsFoundViaMixinDirs('custom-mixins/other.js', + ['./custom-mixins']); }); - expect(instructions.mixins[0].sourceFile).to.eql(appJS); - } + it('resolves relative path in `mixinDirs` option', function() { + verifyMixinIsFoundViaMixinDirs('custom-mixins/other.js', + ['./custom-mixins']); + }); - it('supports `mixinDirs` option', function() { - verifyMixinIsFoundViaMixinDirs('mixins/other.js', ['./mixins']); + it('resolves module relative path in `mixinDirs` option', function() { + verifyMixinIsFoundViaMixinDirs('node_modules/custom-mixins/other.js', + ['custom-mixins']); + }); }); - it('resolves relative path in `mixinDirs` option', function() { - verifyMixinIsFoundViaMixinDirs('custom-mixins/vehicle.js', - ['./custom-mixins']); - }); + describe(' - mixinSources', function() { + beforeEach(function() { + appdir.createConfigFilesSync({}, {}, { + Car: { dataSource: 'db' } + }); + appdir.writeConfigFileSync('models/car.json', { + name: 'Car', + mixins: {'TimeStamps': {} } + }); + }); - it('resolves module relative path in `mixinDirs` option', function() { - verifyMixinIsFoundViaMixinDirs('node_modules/custom-mixins/vehicle.js', - ['custom-mixins']); + function verifyMixinIsFoundViaMixinSources(sourceFile, mixinSources) { + var appJS = appdir.writeFileSync(sourceFile, ''); + + var instructions = boot.compile({ + appRootDir: appdir.PATH, + mixinSources: mixinSources + }); + + expect(instructions.mixins[0].sourceFile).to.eql(appJS); + } + + it('supports `mixinSources` option', function() { + verifyMixinIsFoundViaMixinSources('mixins/time-stamps.js', + ['./mixins']); + }); + + it('resolves relative path in `mixinSources` option', function() { + verifyMixinIsFoundViaMixinSources('custom-mixins/time-stamps.js', + ['./custom-mixins']); + }); + + it('resolves module relative path in `mixinSources` option', + function() { + verifyMixinIsFoundViaMixinSources( + 'node_modules/custom-mixins/time-stamps.js', + ['custom-mixins']); + }); + + it('supports `mixins` option in `model-config.json`', function() { + appdir.createConfigFilesSync({}, {}, { + _meta: { + mixins: ['./custom-mixins'] + }, + Car: { + dataSource: 'db' + } + }); + + var appJS = appdir.writeFileSync('custom-mixins/time-stamps.js', ''); + var instructions = boot.compile(appdir.PATH); + expect(instructions.mixins[0].sourceFile).to.eql(appJS); + }); + + it('sets by default `mixinSources` to `mixins` directory', function() { + var appJS = appdir.writeFileSync('mixins/time-stamps.js', ''); + var instructions = boot.compile(appdir.PATH); + expect(instructions.mixins[0].sourceFile).to.eql(appJS); + }); + + it('loads only mixins used by models', function() { + var appJS = appdir.writeFileSync('mixins/time-stamps.js', ''); + appdir.writeFileSync('mixins/foo.js', ''); + + var instructions = boot.compile(appdir.PATH); + expect(instructions.mixins).to.have.length(1); + expect(instructions.mixins[0].sourceFile).to.eql(appJS); + }); + + it('loads mixins from model using mixin name in JSON file', function() { + var appJS = appdir.writeFileSync('mixins/time-stamps.js', ''); + appdir.writeConfigFileSync('mixins/time-stamps.json', { + name: 'Timestamping' + }); + + appdir.writeConfigFileSync('models/car.json', { + name: 'Car', + mixins: {'Timestamping': {} } + }); + + var instructions = boot.compile(appdir.PATH); + expect(instructions.mixins).to.have.length(1); + expect(instructions.mixins[0].sourceFile).to.eql(appJS); + }); + + it('loads mixin only once for dirs common to mixinDirs & mixinSources', + function() { + var appJS = appdir.writeFileSync('custom-mixins/time-stamps.js', ''); + + var options = { + appRootDir: appdir.PATH, + mixinDirs: ['./custom-mixins'], + mixinSources: ['./custom-mixins'] + }; + + var instructions = boot.compile(options); + expect(instructions.mixins).to.have.length(1); + expect(instructions.mixins[0].sourceFile).to.eql(appJS); + }); + + it('loads mixin from mixinSources, when it is also found in mixinDirs', + function() { + appdir.writeFileSync('mixinDir/time-stamps.js', ''); + var appJS = appdir.writeFileSync('mixinSource/time-stamps.js', ''); + + var options = { + appRootDir: appdir.PATH, + mixinDirs: ['./mixinDir'], + mixinSources: ['./mixinSource'] + }; + + var instructions = boot.compile(options); + expect(instructions.mixins).to.have.length(1); + expect(instructions.mixins[0].sourceFile).to.eql(appJS); + }); + + it('loads mixin from the most recent mixin definition', function() { + appdir.writeFileSync('mixins1/time-stamps.js', ''); + var mixins2 = appdir.writeFileSync('mixins2/time-stamps.js', ''); + + var options = { + appRootDir: appdir.PATH, + mixinSources: ['./mixins1', './mixins2'] + }; + + var instructions = boot.compile(options); + expect(instructions.mixins).to.have.length(1); + expect(instructions.mixins[0].sourceFile).to.eql(mixins2); + }); }); describe('name normalization', function() { var options; beforeEach(function() { - options = { appRootDir: appdir.PATH, mixinDirs: ['./mixins'] }; + options = { appRootDir: appdir.PATH, mixinDirs: ['./custom-mixins'] }; - appdir.writeFileSync('mixins/foo.js', ''); - appdir.writeFileSync('mixins/time-stamps.js', ''); - appdir.writeFileSync('mixins/camelCase.js', ''); - appdir.writeFileSync('mixins/PascalCase.js', ''); - appdir.writeFileSync('mixins/space name.js', ''); + appdir.writeFileSync('custom-mixins/foo.js', ''); + appdir.writeFileSync('custom-mixins/time-stamps.js', ''); + appdir.writeFileSync('custom-mixins/camelCase.js', ''); + appdir.writeFileSync('custom-mixins/PascalCase.js', ''); + appdir.writeFileSync('custom-mixins/space name.js', ''); }); it('supports classify', function() { @@ -1146,15 +1280,15 @@ describe('compiler', function() { }); it('extends definition from JSON with same file name', function() { - var appJS = appdir.writeFileSync('mixins/foo-bar.js', ''); + var appJS = appdir.writeFileSync('custom-mixins/foo-bar.js', ''); - appdir.writeConfigFileSync('mixins/foo-bar.json', { + appdir.writeConfigFileSync('custom-mixins/foo-bar.json', { description: 'JSON file name same as JS file name' }); - appdir.writeConfigFileSync('mixins/FooBar.json', { + appdir.writeConfigFileSync('custom-mixins/FooBar.json', { description: 'JSON file name same as normalized name of mixin' }); var options = { appRootDir: appdir.PATH, - mixinDirs: ['./mixins'], + mixinDirs: ['./custom-mixins'], normalization: 'classify' }; var instructions = boot.compile(options); @@ -1166,7 +1300,6 @@ describe('compiler', function() { } ]); }); - }); }); diff --git a/test/executor.test.js b/test/executor.test.js index 850e46e..997c253 100644 --- a/test/executor.test.js +++ b/test/executor.test.js @@ -293,28 +293,39 @@ describe('executor', function() { }); describe ('for mixins', function() { - it('defines mixins from instructions', function() { - appdir.writeFileSync('mixins/example.js', + var options; + beforeEach(function() { + appdir.writeFileSync('custom-mixins/example.js', 'module.exports = ' + 'function(Model, options) {}'); - appdir.writeFileSync('mixins/time-stamps.js', + appdir.writeFileSync('custom-mixins/time-stamps.js', 'module.exports = ' + 'function(Model, options) {}'); - appdir.writeConfigFileSync('mixins/time-stamps.json', { + appdir.writeConfigFileSync('custom-mixins/time-stamps.json', { name: 'Timestamping' }); - var options = { - appRootDir: appdir.PATH, - mixinDirs: ['./mixins'] + options = { + appRootDir: appdir.PATH }; + }); + it('defines mixins from instructions - using `mixinDirs`', function() { + options.mixinDirs = ['./custom-mixins']; boot(app, options); var modelBuilder = app.registry.modelBuilder; + var registry = modelBuilder.mixins.mixins; + expect(Object.keys(registry)).to.eql(['Example', 'Timestamping']); + }); + it('defines mixins from instructions - using `mixinSources`', function() { + options.mixinSources = ['./custom-mixins']; + boot(app, options); + + var modelBuilder = app.registry.modelBuilder; var registry = modelBuilder.mixins.mixins; expect(Object.keys(registry)).to.eql(['Example', 'Timestamping']); });