From b779a345e8dac4c658a1816ba595a6b95db47fa2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Raphael=20von=20der=20Gr=C3=BCn?= Date: Thu, 9 Jan 2020 23:23:39 +0100 Subject: [PATCH] refactor(PluginInfo): cleanup & simplify (#132) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: PluginInfo * _getTags: simplify * _getTagsInPlatform: support a list of platforms * getPodSpecs: remove useless undefined value filtering * getPodSpecs: further improve formatting * getPreferences: simplify * getHookScripts: simplify * getFrameworks: only determine vars once * getFrameworks: improve readability * ctor: improve * ctor: better var name for root elem * destructure xml-helpers * inline addCordova helper * apply lebab.destruct-params * convert comments to JSDocs * remove outdated TODO; sync is faster * map tags explicitly * make _getTags* private methods Co-authored-by: エリス --- src/PluginInfo/PluginInfo.js | 485 +++++++++++++++++++---------------- 1 file changed, 261 insertions(+), 224 deletions(-) diff --git a/src/PluginInfo/PluginInfo.js b/src/PluginInfo/PluginInfo.js index f54509be..d3606647 100644 --- a/src/PluginInfo/PluginInfo.js +++ b/src/PluginInfo/PluginInfo.js @@ -17,101 +17,112 @@ under the License. */ -/* -A class for holidng the information currently stored in plugin.xml -It should also be able to answer questions like whether the plugin -is compatible with a given engine version. - -TODO (kamrik): refactor this to not use sync functions and return promises. -*/ - const path = require('path'); const fs = require('fs-extra'); -const xml_helpers = require('../util/xml-helpers'); +const { parseElementtreeSync } = require('../util/xml-helpers'); const CordovaError = require('../CordovaError'); +/** + * A class for holding the information currently stored in plugin.xml + * + * It should also be able to answer questions like whether the plugin + * is compatible with a given engine version. + */ class PluginInfo { constructor (dirname) { + this.dir = dirname; this.filepath = path.join(dirname, 'plugin.xml'); + if (!fs.existsSync(this.filepath)) { throw new CordovaError(`Cannot find plugin.xml for plugin "${path.basename(dirname)}". Please try adding it again.`); } - this.dir = dirname; - const et = this._et = xml_helpers.parseElementtreeSync(this.filepath); - const pelem = et.getroot(); - this.id = pelem.attrib.id; - this.version = pelem.attrib.version; + this._et = parseElementtreeSync(this.filepath); + const root = this._et.getroot(); + + this.id = root.attrib.id; + this.version = root.attrib.version; // Optional fields - this.name = pelem.findtext('name'); - this.description = pelem.findtext('description'); - this.license = pelem.findtext('license'); - this.repo = pelem.findtext('repo'); - this.issue = pelem.findtext('issue'); - this.keywords = pelem.findtext('keywords'); - this.info = pelem.findtext('info'); - if (this.keywords) { - this.keywords = this.keywords.split(',').map(s => s.trim()); + const optTags = 'name description license repo issue info'.split(' '); + for (const tag of optTags) { + this[tag] = root.findtext(tag); } + + const keywordText = root.findtext('keywords'); + this.keywords = keywordText && keywordText.split(',').map(s => s.trim()); } - // tag - // Example: - // Used to require a variable to be specified via --variable when installing the plugin. - // returns { key : default | null} + /** + * tag + * + * Used to require a variable to be specified via --variable when installing the plugin. + * + * @example + * + * @param {string} platform + * @return {Object} { key : default | null} + */ getPreferences (platform) { - return _getTags(this._et, 'preference', platform, prefTag => { - const name = prefTag.attrib.name.toUpperCase(); - const def = prefTag.attrib.default || null; - return { preference: name, default: def }; - }) - .reduce((preferences, pref) => { - preferences[pref.preference] = pref.default; - return preferences; - }, {}); + return this._getTags('preference', platform).map(({ attrib }) => ({ + [attrib.name.toUpperCase()]: attrib.default || null + })) + .reduce((acc, pref) => Object.assign(acc, pref), {}); } - // + /** + * + * + * @param {string} platform + */ getAssets (platform) { - return _getTags(this._et, 'asset', platform, tag => { - const src = tag.attrib.src; - const target = tag.attrib.target; + return this._getTags('asset', platform).map(({ attrib }) => { + const src = attrib.src; + const target = attrib.target; if (!src || !target) { - throw new Error(`Malformed tag. Both "src" and "target" attributes must be specified in\n${this.filepath}`); + throw new Error(`Malformed tag. Both "src" and "target" attributes must be specified in ${this.filepath}`); } return { itemType: 'asset', src, target }; }); } - // - // Example: - // + /** + * + * + * @example + * + * + * @param {string} platform + */ getDependencies (platform) { - return _getTags(this._et, 'dependency', platform, tag => { - if (!tag.attrib.id) { + return this._getTags('dependency', platform).map(({ attrib }) => { + if (!attrib.id) { throw new CordovaError(` tag is missing id attribute in ${this.filepath}`); } return { - id: tag.attrib.id, - version: tag.attrib.version || '', - url: tag.attrib.url || '', - subdir: tag.attrib.subdir || '', - commit: tag.attrib.commit, - git_ref: tag.attrib.commit + id: attrib.id, + version: attrib.version || '', + url: attrib.url || '', + subdir: attrib.subdir || '', + commit: attrib.commit, + git_ref: attrib.commit }; }); } - // tag + /** + * tag + * + * @param {string} platform + */ getConfigFiles (platform) { - return _getTags(this._et, 'config-file', platform, tag => ({ + return this._getTags('config-file', platform).map(tag => ({ target: tag.attrib.target, parent: tag.attrib.parent, after: tag.attrib.after, @@ -122,8 +133,13 @@ class PluginInfo { })); } + /** + * tag + * + * @param {string} platform + */ getEditConfigs (platform) { - return _getTags(this._et, 'edit-config', platform, tag => ({ + return this._getTags('edit-config', platform).map(tag => ({ file: tag.attrib.file, target: tag.attrib.target, mode: tag.attrib.mode, @@ -131,137 +147,163 @@ class PluginInfo { })); } - // tags, both global and within a + /** + * tags, both global and within a + * + * @param {string} platform + */ // TODO (kamrik): Do we ever use under ? Example wanted. getInfo (platform) { - return _getTags(this._et, 'info', platform, elem => elem.text) + return this._getTags('info', platform).map(elem => elem.text) // Filter out any undefined or empty strings. .filter(Boolean); } - // - // Examples: - // - // + /** + * + * + * @example + * + * + * + * @param {string} platform + */ getSourceFiles (platform) { - return _getTagsInPlatform(this._et, 'source-file', platform, tag => ({ + return this._getTagsInPlatform('source-file', platform).map(({ attrib }) => ({ itemType: 'source-file', - src: tag.attrib.src, - framework: isStrTrue(tag.attrib.framework), - weak: isStrTrue(tag.attrib.weak), - compilerFlags: tag.attrib['compiler-flags'], - targetDir: tag.attrib['target-dir'] + src: attrib.src, + framework: isStrTrue(attrib.framework), + weak: isStrTrue(attrib.weak), + compilerFlags: attrib['compiler-flags'], + targetDir: attrib['target-dir'] })); } - // - // Example: - // + /** + * + * + * @example + * + * @param {string} platform + */ getHeaderFiles (platform) { - return _getTagsInPlatform(this._et, 'header-file', platform, tag => ({ + return this._getTagsInPlatform('header-file', platform).map(({ attrib }) => ({ itemType: 'header-file', - src: tag.attrib.src, - targetDir: tag.attrib['target-dir'], - type: tag.attrib.type + src: attrib.src, + targetDir: attrib['target-dir'], + type: attrib.type })); } - // - // Example: - // + /** + * + * + * @example + * + * + * @param {string} platform + */ getResourceFiles (platform) { - return _getTagsInPlatform(this._et, 'resource-file', platform, tag => ({ + return this._getTagsInPlatform('resource-file', platform).map(({ attrib }) => ({ itemType: 'resource-file', - src: tag.attrib.src, - target: tag.attrib.target, - versions: tag.attrib.versions, - deviceTarget: tag.attrib['device-target'], - arch: tag.attrib.arch, - reference: tag.attrib.reference + src: attrib.src, + target: attrib.target, + versions: attrib.versions, + deviceTarget: attrib['device-target'], + arch: attrib.arch, + reference: attrib.reference })); } - // - // Example: - // + /** + * + * + * @example + * + * + * @param {string} platform + */ getLibFiles (platform) { - return _getTagsInPlatform(this._et, 'lib-file', platform, tag => ({ + return this._getTagsInPlatform('lib-file', platform).map(({ attrib }) => ({ itemType: 'lib-file', - src: tag.attrib.src, - arch: tag.attrib.arch, - Include: tag.attrib.Include, - versions: tag.attrib.versions, - deviceTarget: tag.attrib['device-target'] || tag.attrib.target + src: attrib.src, + arch: attrib.arch, + Include: attrib.Include, + versions: attrib.versions, + deviceTarget: attrib['device-target'] || attrib.target })); } - // - // Example: - // - // - // - // - // - // - // - // - // - // - // - // - // - // + /** + * + * + * @example + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * @param {string} platform + */ getPodSpecs (platform) { - return _getTagsInPlatform(this._et, 'podspec', platform, tag => { - let declarations = null; - let sources = null; - let libraries = null; + return this._getTagsInPlatform('podspec', platform).map(tag => { const config = tag.find('config'); const pods = tag.find('pods'); - if (config != null) { - sources = config.findall('source').map(t => ({ - url: t.attrib.url - })).reduce((acc, val) => { - return Object.assign({}, acc, { [val.url]: { source: val.url } }); - }, {}); - } - if (pods != null) { - declarations = Object.keys(pods.attrib).reduce((acc, key) => { - return pods.attrib[key] === undefined ? acc : Object.assign({}, acc, { [key]: pods.attrib[key] }); - }, {}); - libraries = pods.findall('pod').map(t => { - return Object.keys(t.attrib).reduce((acc, key) => { - return t.attrib[key] === undefined ? acc : Object.assign({}, acc, { [key]: t.attrib[key] }); - }, {}); - }).reduce((acc, val) => { - return Object.assign({}, acc, { [val.name]: val }); - }, {}); - } + + const sources = config && config.findall('source') + .map(el => ({ source: el.attrib.url })) + .reduce((acc, val) => Object.assign(acc, { [val.source]: val }), {}); + + const declarations = pods && pods.attrib; + + const libraries = pods && pods.findall('pod') + .map(t => t.attrib) + .reduce((acc, val) => Object.assign(acc, { [val.name]: val }), {}); + return { declarations, sources, libraries }; }); } - // - // Example: - // + /** + * + * + * @example + * + * + * @param {string} hook + * @param {string} platforms + */ getHookScripts (hook, platforms) { - let scriptElements = this._et.findall('./hook'); - - if (platforms) { - platforms.forEach(platform => { - scriptElements = scriptElements.concat(this._et.findall(`./platform[@name="${platform}"]/hook`)); - }); - } - - function filterScriptByHookType (el) { - return el.attrib.src && el.attrib.type && el.attrib.type.toLowerCase() === hook; - } - - return scriptElements.filter(filterScriptByHookType); + return this._getTags('hook', platforms) + .filter(({ attrib }) => + attrib.src && attrib.type && + attrib.type.toLowerCase() === hook + ); } + /** + * + * + * @param {string} platform + */ getJsModules (platform) { - return _getTags(this._et, 'js-module', platform, tag => ({ + return this._getTags('js-module', platform).map(tag => ({ itemType: 'js-module', name: tag.attrib.name, src: tag.attrib.src, @@ -272,18 +314,16 @@ class PluginInfo { } getEngines () { - return this._et.findall('engines/engine').map(n => ({ - name: n.attrib.name, - version: n.attrib.version, - platform: n.attrib.platform, - scriptSrc: n.attrib.scriptSrc + return this._et.findall('engines/engine').map(({ attrib }) => ({ + name: attrib.name, + version: attrib.version, + platform: attrib.platform, + scriptSrc: attrib.scriptSrc })); } getPlatforms () { - return this._et.findall('platform').map(n => ({ - name: n.attrib.name - })); + return this._et.findall('platform').map(n => ({ name: n.attrib.name })); } getPlatformsArray () { @@ -291,41 +331,36 @@ class PluginInfo { } getFrameworks (platform, options) { - return _getTags(this._et, 'framework', platform, el => { - let src = el.attrib.src; - if (options) { - let vars = options.cli_variables || {}; - - if (Object.keys(vars).length === 0) { - // get variable defaults from plugin.xml for removal - vars = this.getPreferences(platform); - } - let regExp; - // Iterate over plugin variables. - // Replace them in framework src if they exist - Object.keys(vars).forEach(name => { - if (vars[name]) { - regExp = new RegExp(`\\$${name}`, 'g'); - src = src.replace(regExp, vars[name]); - } - }); - } - return { - itemType: 'framework', - type: el.attrib.type, - parent: el.attrib.parent, - custom: isStrTrue(el.attrib.custom), - embed: isStrTrue(el.attrib.embed), - src, - spec: el.attrib.spec, - weak: isStrTrue(el.attrib.weak), - versions: el.attrib.versions, - targetDir: el.attrib['target-dir'], - deviceTarget: el.attrib['device-target'] || el.attrib.target, - arch: el.attrib.arch, - implementation: el.attrib.implementation - }; - }); + const { cli_variables = {} } = options || {}; + + const vars = Object.keys(cli_variables).length === 0 + ? this.getPreferences(platform) + : cli_variables; + + const varExpansions = Object.entries(vars) + .filter(([, value]) => value) + .map(([name, value]) => + s => s.replace(new RegExp(`\\$${name}`, 'g'), value) + ); + + // Replaces plugin variables in s if they exist + const expandVars = s => varExpansions.reduce((acc, fn) => fn(acc), s); + + return this._getTags('framework', platform).map(({ attrib }) => ({ + itemType: 'framework', + type: attrib.type, + parent: attrib.parent, + custom: isStrTrue(attrib.custom), + embed: isStrTrue(attrib.embed), + src: expandVars(attrib.src), + spec: attrib.spec, + weak: isStrTrue(attrib.weak), + versions: attrib.versions, + targetDir: attrib['target-dir'], + deviceTarget: attrib['device-target'] || attrib.target, + arch: attrib.arch, + implementation: attrib.implementation + })); } getFilesAndFrameworks (platform, options) { @@ -343,40 +378,41 @@ class PluginInfo { getKeywordsAndPlatforms () { return (this.keywords || []) .concat('ecosystem:cordova') - .concat(addCordova(this.getPlatformsArray())); + .concat(this.getPlatformsArray().map(p => `cordova-${p}`)); } -} -// Helper function used to prefix every element of an array with cordova- -// Useful when we want to modify platforms to be cordova-platform -function addCordova (someArray) { - return someArray.map(element => `cordova-${element}`); -} - -// Helper function used by most of the getSomething methods of PluginInfo. -// Get all elements of a given name. Both in root and in platform sections -// for the given platform. If transform is given and is a function, it is -// applied to each element. -function _getTags (pelem, tag, platform, transform) { - const platformTag = pelem.find(`./platform[@name="${platform}"]`); - let tagsInRoot = pelem.findall(tag); - tagsInRoot = tagsInRoot || []; - const tagsInPlatform = platformTag ? platformTag.findall(tag) : []; - let tags = tagsInRoot.concat(tagsInPlatform); - if (typeof transform === 'function') { - tags = tags.map(transform); + /** + * Helper method used by most of the getSomething methods of PluginInfo. + * + * Get all elements of a given name. Both in root and in platform sections + * for the given platform. + * + * @private + * + * @param {string} tag + * @param {string|string[]} platform + */ + _getTags (tag, platform) { + return this._et.findall(tag) + .concat(this._getTagsInPlatform(tag, platform)); } - return tags; -} -// Same as _getTags() but only looks inside a platform section. -function _getTagsInPlatform (pelem, tag, platform, transform) { - const platformTag = pelem.find(`./platform[@name="${platform}"]`); - let tags = platformTag ? platformTag.findall(tag) : []; - if (typeof transform === 'function') { - tags = tags.map(transform); + /** + * Same as _getTags() but only looks inside a platform section. + * + * @private + * + * @param {string} tag + * @param {string|string[]} platform + */ + _getTagsInPlatform (tag, platform) { + const platforms = [].concat(platform); + + return [].concat(...platforms.map(platform => { + const platformTag = this._et.find(`./platform[@name="${platform}"]`); + return platformTag ? platformTag.findall(tag) : []; + })); } - return tags; } // Check if x is a string 'true'. @@ -385,6 +421,7 @@ function isStrTrue (x) { } module.exports = PluginInfo; + // Backwards compat: PluginInfo.PluginInfo = PluginInfo; PluginInfo.loadPluginsDir = dir => {