You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by dp...@apache.org on 2019/03/29 16:44:10 UTC
[cordova-lib] branch master updated: Fix restoring plugins from
package.json
This is an automated email from the ASF dual-hosted git repository.
dpogue pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cordova-lib.git
The following commit(s) were added to refs/heads/master by this push:
new 677b092 Fix restoring plugins from package.json
677b092 is described below
commit 677b092670d089066c92d0ad824492df802c4582
Author: Darryl Pogue <da...@dpogue.ca>
AuthorDate: Fri Mar 29 09:44:05 2019 -0700
Fix restoring plugins from package.json
Some of the code removed in GH-750 had side effects on the rest of the
plugin restoration procedure. Namely, despite wanting to migrate to
package.json, we actually relied on config.xml for retrieving the plugin
spec and variables. When we stopped syncing changes back to config.xml,
those plugins stopped getting restored.
I've refactored this significantly to reduce complexity and make it
easier to read and understand.
The existing tests did neither describe nor test the new behavior
sufficiently.
---
spec/cordova/restore-util.spec.js | 211 +++++++++++++++-----------------------
src/cordova/restore-util.js | 180 ++++++++++++--------------------
src/cordova/util.js | 19 +++-
3 files changed, 164 insertions(+), 246 deletions(-)
diff --git a/spec/cordova/restore-util.spec.js b/spec/cordova/restore-util.spec.js
index 17291e5..0097968 100644
--- a/spec/cordova/restore-util.spec.js
+++ b/spec/cordova/restore-util.spec.js
@@ -124,12 +124,13 @@ describe('cordova/restore-util', () => {
// Check that dependencies key in package.json contains the expected specs
// We only check the specs for plugins where an expected spec was given
- const specs = plugins.reduce((o, { name, spec }) => {
+ const expectedSpecs = plugins.reduce((o, { name, spec }) => {
if (spec) o[name] = spec;
return o;
}, {});
- if (Object.keys(specs).length > 0) {
- expect(pkgJson.dependencies).toEqual(jasmine.objectContaining(specs));
+ if (Object.keys(expectedSpecs).length > 0) {
+ let specs = Object.assign({}, pkgJson.dependencies, pkgJson.devDependencies);
+ expect(specs).toEqual(jasmine.objectContaining(expectedSpecs));
}
}
@@ -138,22 +139,23 @@ describe('cordova/restore-util', () => {
}
function expectPluginAdded (plugin) {
+ const expectedOpts = {
+ plugins: jasmine.arrayContaining([
+ jasmine.stringMatching(plugin.name)
+ ])
+ };
+ if ('variables' in plugin) {
+ expectedOpts.cli_variables = plugin.variables;
+ }
expect(cordovaPlugin.add).toHaveBeenCalledWith(
jasmine.any(String), jasmine.anything(),
- jasmine.objectContaining({
- plugins: jasmine.arrayContaining([
- jasmine.stringMatching(plugin)
- ])
- })
+ jasmine.objectContaining(expectedOpts)
);
}
- function expectConsistentPlugins (plugins) {
- const unwrappedPlugins = plugins.map(p => p.sample || p);
- expectPluginsInPkgJson(unwrappedPlugins);
-
- const pluginNames = unwrappedPlugins.map(({ name }) => name);
- pluginNames.forEach(expectPluginAdded);
+ function expectPluginsAddedAndSavedToPkgJson (plugins) {
+ expectPluginsInPkgJson(plugins);
+ plugins.forEach(expectPluginAdded);
}
describe('installPlatformsFromConfigXML', () => {
@@ -264,165 +266,122 @@ describe('cordova/restore-util', () => {
describe('installPluginsFromConfigXML', () => {
beforeEach(() => {
// Add some platform to test the plugins with
- getCfg()
- .addEngine(testPlatform)
- .write();
setPkgJson('cordova.platforms', [testPlatform]);
});
- /**
- * When pkg.json and config.xml define different values for a plugin variable,
- * pkg.json should win and that value will be used to replace config's value.
- */
- it('Test#011 : updates config.xml to use the variable found in pkg.json', () => {
- getCfg()
- .addPlugin({
- name: 'cordova-plugin-camera',
- variables: { variable_1: 'config' }
- })
- .write();
+ it('Test#011 : restores saved plugin', () => {
+ setPkgJson('dependencies', {
+ 'cordova-plugin-camera': '^2.3.0'
+ });
setPkgJson('cordova.plugins', {
- 'cordova-plugin-camera': { variable_1: 'json' }
+ 'cordova-plugin-camera': { variable_1: 'value_1' }
});
return restore.installPluginsFromConfigXML({ save: true }).then(() => {
- expectConsistentPlugins([
- jasmine.objectContaining({
- name: 'cordova-plugin-camera',
- variables: { variable_1: 'json' }
- })
- ]);
+ expectPluginAdded({
+ name: 'cordova-plugin-camera',
+ spec: '^2.3.0',
+ variables: { variable_1: 'value_1' }
+ });
});
});
- /**
- * When config.xml and pkg.json share a common plugin but pkg.json defines no variables for it,
- * prepare will update pkg.json to match config.xml's plugins/variables.
- */
- it('Test#012 : update pkg.json to include plugin and variable found in config.xml', () => {
- getCfg()
- .addPlugin({
- name: 'cordova-plugin-camera',
- variables: { variable_1: 'value_1' }
- })
- .write();
- setPkgJson('cordova.plugins', {
- 'cordova-plugin-camera': {}
+ it('Test#012 : restores saved plugin using an URL spec', () => {
+ const PLUGIN_ID = 'cordova-plugin-splashscreen';
+ const PLUGIN_URL = 'https://github.com/apache/cordova-plugin-splashscreen';
+
+ setPkgJson('dependencies', {
+ [PLUGIN_ID]: `git+${PLUGIN_URL}.git`
+ });
+ setPkgJson('cordova.plugins', { [PLUGIN_ID]: {} });
+
+ return restore.installPluginsFromConfigXML({ save: true }).then(() => {
+ expectPluginAdded({
+ name: PLUGIN_ID,
+ spec: `git+${PLUGIN_URL}.git`,
+ variables: {}
+ });
});
+ });
+
+ it('Test#013 : does NOT detect plugins from dependencies ', () => {
+ setPkgJson('dependencies', { 'cordova-plugin-device': '~1.0.0' });
+ setPkgJson('devDependencies', { 'cordova-plugin-camera': '~1.0.0' });
return restore.installPluginsFromConfigXML({ save: true }).then(() => {
- expectConsistentPlugins([
- jasmine.objectContaining({
- name: 'cordova-plugin-camera',
- variables: { variable_1: 'value_1' }
- })
- ]);
+ expect(cordovaPlugin.add).not.toHaveBeenCalled();
});
});
- /**
- * For plugins that are the same, it will merge their variables together for the final list.
- * Plugins that are unique to that file, will be copied over to the file that is missing it.
- * Config.xml and pkg.json will have identical plugins and variables after cordova prepare.
- */
- it('Test#013 : update pkg.json AND config.xml to include all plugins and merge unique variables', () => {
+ it('Test#014 : adds any plugins only present in config.xml to pkg.json', () => {
getCfg()
.addPlugin({
- name: 'cordova-plugin-camera',
- variables: { variable_3: 'value_3' }
- })
- .addPlugin({
- name: 'cordova-plugin-splashscreen',
+ name: 'cordova-plugin-device',
+ spec: '~1.0.0',
variables: {}
})
.write();
- setPkgJson('cordova.plugins', {
- 'cordova-plugin-splashscreen': {},
- 'cordova-plugin-camera': { variable_1: ' ', variable_2: ' ' },
- 'cordova-plugin-device': { variable_1: 'value_1' }
- });
+
+ setPkgJson('cordova.plugins', { 'cordova-plugin-camera': {} });
+ setPkgJson('devDependencies', { 'cordova-plugin-camera': '^2.3.0' });
return restore.installPluginsFromConfigXML({ save: true }).then(() => {
- expectPluginsInPkgJson([
- jasmine.objectContaining({
- name: 'cordova-plugin-camera',
- variables: { variable_1: ' ', variable_2: ' ', variable_3: 'value_3' }
- }),
- jasmine.objectContaining({
- name: 'cordova-plugin-splashscreen',
- variables: {}
- }),
- jasmine.objectContaining({
- name: 'cordova-plugin-device',
- variables: { variable_1: 'value_1' }
- })
- ].map(p => p.sample || p));
+ expectPluginsAddedAndSavedToPkgJson([{
+ name: 'cordova-plugin-camera',
+ spec: '^2.3.0',
+ variables: {}
+ }, {
+ name: 'cordova-plugin-device',
+ spec: '~1.0.0',
+ variables: {}
+ }]);
});
});
- /**
- * If either file is missing a plugin, it will be added with the correct variables.
- * If there is a matching plugin name, the variables will be merged and then added
- * to config and pkg.json.
- */
- it('Test#014 : update pkg.json AND config.xml to include all plugins and merge variables (no dupes)', () => {
+ it('Test#015 : prefers pkg.json plugins over those from config.xml', () => {
getCfg()
.addPlugin({
name: 'cordova-plugin-camera',
spec: '~2.2.0',
- variables: { variable_1: 'value_1', variable_2: 'value_2' }
- })
- .addPlugin({
- name: 'cordova-plugin-device',
- spec: '~1.0.0',
- variables: {}
+ variables: { common_var: 'xml', xml_var: 'foo' }
})
.write();
- setPkgJson('dependencies', {
- 'cordova-plugin-camera': '^2.3.0'
- });
+
setPkgJson('cordova.plugins', {
- 'cordova-plugin-splashscreen': {},
- 'cordova-plugin-camera': { variable_1: 'value_1', variable_3: 'value_3' }
+ 'cordova-plugin-camera': { common_var: 'json', json_var: 'foo' }
});
+ setPkgJson('devDependencies', { 'cordova-plugin-camera': '^2.3.0' });
return restore.installPluginsFromConfigXML({ save: true }).then(() => {
- expectPluginsInPkgJson([{
+ expectPluginsAddedAndSavedToPkgJson([{
name: 'cordova-plugin-camera',
spec: '^2.3.0',
- variables: { variable_1: 'value_1', variable_2: 'value_2', variable_3: 'value_3' }
- }, {
- name: 'cordova-plugin-device',
- spec: '~1.0.0',
- variables: {}
- }, {
- name: 'cordova-plugin-splashscreen',
- spec: undefined,
- variables: {}
- }].map(p => p.sample || p));
+ variables: { common_var: 'json', json_var: 'foo' }
+ }]);
});
});
- it('Test#018 : should restore saved plugin using an URL spec', () => {
- const PLUGIN_ID = 'cordova-plugin-splashscreen';
- const PLUGIN_URL = 'https://github.com/apache/cordova-plugin-splashscreen';
-
+ it('Test#018 : does NOT produce conflicting dependencies', () => {
getCfg()
- .addPlugin({ name: PLUGIN_ID, spec: PLUGIN_URL })
+ .addPlugin({
+ name: 'cordova-plugin-camera',
+ spec: '~2.2.0',
+ variables: { common_var: 'xml', xml_var: 'foo' }
+ })
.write();
- setPkgJson('dependencies', {
- [PLUGIN_ID]: `git+${PLUGIN_URL}.git`
- });
- setPkgJson('cordova.plugins', { [PLUGIN_ID]: {} });
+ setPkgJson('dependencies', { 'cordova-plugin-camera': '^2.3.0' });
return restore.installPluginsFromConfigXML({ save: true }).then(() => {
- // Config.xml spec now matches the one in pkg.json.
- expectConsistentPlugins([{
- name: PLUGIN_ID,
- spec: `git+${PLUGIN_URL}.git`,
- variables: jasmine.any(Object)
+ expectPluginsAddedAndSavedToPkgJson([{
+ name: 'cordova-plugin-camera',
+ spec: '^2.3.0',
+ variables: { common_var: 'xml', xml_var: 'foo' }
}]);
+
+ const pluginOccurences = !!getPkgJson('dependencies.cordova-plugin-camera')
+ + !!getPkgJson('devDependencies.cordova-plugin-camera');
+ expect(pluginOccurences).toBe(1);
});
});
});
diff --git a/src/cordova/restore-util.js b/src/cordova/restore-util.js
index 0cf21b1..58d27ad 100644
--- a/src/cordova/restore-util.js
+++ b/src/cordova/restore-util.js
@@ -213,157 +213,103 @@ function installPlatformsFromConfigXML (platforms, opts) {
// Returns a promise.
function installPluginsFromConfigXML (args) {
- events.emit('verbose', 'Checking config.xml for saved plugins that haven\'t been added to the project');
- // Install plugins that are listed in config.xml.
- var projectRoot = cordova_util.cdProjectRoot();
- var configPath = cordova_util.projectConfig(projectRoot);
- var cfg = new ConfigParser(configPath);
- var plugins_dir = path.join(projectRoot, 'plugins');
- var pkgJsonPath = path.join(projectRoot, 'package.json');
- var pkgJson;
- var modifiedPkgJson = false;
- var comboObject;
- var mergedPluginSpecs = {};
- var comboPluginIdArray;
- var configPlugin;
- var configPluginVariables;
- var pkgJsonPluginVariables;
- var key;
+ events.emit('verbose', 'Checking for saved plugins that haven\'t been added to the project');
+
+ const projectRoot = cordova_util.getProjectRoot();
+ const pluginsRoot = path.join(projectRoot, 'plugins');
+ const pkgJsonPath = path.join(projectRoot, 'package.json');
+ const confXmlPath = cordova_util.projectConfig(projectRoot);
+
+ let pkgJson = {};
+ let indent = ' ';
- // Check if path exists and require pkgJsonPath.
if (fs.existsSync(pkgJsonPath)) {
- pkgJson = require(pkgJsonPath);
+ const fileData = fs.readFileSync(pkgJsonPath, 'utf8');
+ indent = detectIndent(fileData).indent;
+ pkgJson = JSON.parse(fileData);
}
- if (pkgJson !== undefined && pkgJson.cordova !== undefined && pkgJson.cordova.plugins !== undefined) {
- comboPluginIdArray = Object.keys(pkgJson.cordova.plugins);
- // Create a merged plugin data array (comboObject)
- // and add all of the package.json plugins to comboObject.
- comboObject = pkgJson.cordova.plugins;
- } else {
- comboObject = {};
- comboPluginIdArray = [];
- }
+ pkgJson.devDependencies = pkgJson.devDependencies || {};
+ pkgJson.cordova = pkgJson.cordova || {};
+ pkgJson.cordova.plugins = pkgJson.cordova.plugins || {};
- // Get all config.xml plugin ids (names).
- var pluginIdConfig = cfg.getPluginIdList();
- if (pluginIdConfig === undefined) {
- pluginIdConfig = [];
- }
+ const pkgPluginIDs = Object.keys(pkgJson.cordova.plugins);
+ const pkgSpecs = Object.assign({}, pkgJson.dependencies, pkgJson.devDependencies);
- if (pkgJson !== undefined) {
- if (pkgJson.cordova === undefined) {
- pkgJson.cordova = {};
- }
- if (pkgJson.cordova.plugins === undefined) {
- pkgJson.cordova.plugins = {};
- }
+ // Check for plugins listed in config.xml
+ const cfg = new ConfigParser(confXmlPath);
+ const cfgPluginIDs = cfg.getPluginIdList();
- // Check to see which plugins are initially the same in pkg.json and config.xml.
- // Add missing plugin variables in package.json from config.xml.
- comboPluginIdArray.forEach(function (item) {
+ cfgPluginIDs.forEach(plID => {
+ // If package.json includes the plugin, we use that config
+ // Otherwise, we need to add the plugin to package.json
+ if (!pkgPluginIDs.includes(plID)) {
+ events.emit('info', `Plugin '${plID}' found in config.xml... Migrating it to package.json`);
- function includeFunc (container, value) {
- var returnValue = false;
- var pos = container.indexOf(value);
- if (pos >= 0) {
- returnValue = true;
- }
- return returnValue;
- }
- var result = includeFunc(pluginIdConfig, item);
-
- if (result === true) {
- configPlugin = cfg.getPlugin(item);
- configPluginVariables = configPlugin.variables;
- pkgJsonPluginVariables = comboObject[item];
- for (var key in configPluginVariables) {
- // Handle conflicts, package.json wins.
- // Only add the variable to package.json if it doesn't already exist.
- if (pkgJsonPluginVariables[key] === undefined) {
- pkgJsonPluginVariables[key] = configPluginVariables[key];
- comboObject[item][key] = configPluginVariables[key];
- modifiedPkgJson = true;
- }
- }
- }
- // Get the spec from package.json and add it to mergedPluginSpecs.
- if (pkgJson.dependencies && pkgJson.dependencies[item]) {
- mergedPluginSpecs[item] = pkgJson.dependencies[item];
+ const cfgPlugin = cfg.getPlugin(plID);
+
+ // If config.xml has a spec for the plugin and package.json has not,
+ // add the spec to devDependencies of package.json
+ if (cfgPlugin.spec && !(plID in pkgSpecs)) {
+ pkgJson.devDependencies[plID] = cfgPlugin.spec;
}
- });
- // Check to see if pkg.json plugin(id) and config plugin(id) match.
- if (comboPluginIdArray.toString() !== pluginIdConfig.toString()) {
- // If there is a config plugin that does NOT already exist in
- // comboPluginIdArray, add it and its variables.
- pluginIdConfig.forEach(function (item) {
- if (comboPluginIdArray.indexOf(item) < 0) {
- comboPluginIdArray.push(item);
- var configXMLPlugin = cfg.getPlugin(item);
- comboObject[item] = configXMLPlugin.variables;
- modifiedPkgJson = true;
- }
- });
+ pkgJson.cordova.plugins[plID] = Object.assign({}, cfgPlugin.variables);
}
+ });
- // Add specs from config.xml to mergedPluginSpecs.
- pluginIdConfig.forEach(function (item) {
- var configXMLPlugin = cfg.getPlugin(item);
- if (mergedPluginSpecs[item] === undefined && configXMLPlugin.spec) {
- mergedPluginSpecs[item] = configXMLPlugin.spec;
- modifiedPkgJson = true;
- }
+ // Now that plugins have been updated, re-fetch them from package.json
+ const pluginIDs = Object.keys(pkgJson.cordova.plugins);
+
+ if (pluginIDs.length !== pkgPluginIDs.length) {
+ // We've modified package.json and need to save it
+ fs.outputJsonSync(pkgJsonPath, pkgJson, {
+ indent: indent,
+ encoding: 'utf8'
});
- // If pkg.json plugins have been modified, write to it.
- if (modifiedPkgJson === true && args.save !== false) {
- pkgJson.cordova.plugins = comboObject;
- if (pkgJson.dependencies === undefined) {
- pkgJson.dependencies = {};
- }
- for (key in mergedPluginSpecs) {
- pkgJson.dependencies[key] = mergedPluginSpecs[key];
- }
- var file = fs.readFileSync(pkgJsonPath, 'utf8');
- var indent = detectIndent(file).indent || ' ';
- fs.writeFileSync(pkgJsonPath, JSON.stringify(pkgJson, null, indent), 'utf8');
- }
}
- // Intermediate variable to store current installing plugin name
- // to be able to create informative warning on plugin failure
- var pluginName;
+ const specs = Object.assign({}, pkgJson.dependencies, pkgJson.devDependencies);
+
+ const plugins = pluginIDs.map(plID => ({
+ name: plID,
+ spec: specs[plID],
+ variables: pkgJson.cordova.plugins[plID] || {}
+ }));
+
+ let pluginName = '';
+
// CB-9560 : Run `plugin add` serially, one plugin after another
// We need to wait for the plugin and its dependencies to be installed
// before installing the next root plugin otherwise we can have common
// plugin dependencies installed twice which throws a nasty error.
- return promiseutil.Q_chainmap_graceful(pluginIdConfig, function (featureId) {
- var pluginPath = path.join(plugins_dir, featureId);
+ return promiseutil.Q_chainmap_graceful(plugins, function (pluginConfig) {
+ pluginName = pluginConfig.name;
+
+ const pluginPath = path.join(pluginsRoot, pluginName);
if (fs.existsSync(pluginPath)) {
// Plugin already exists
return Promise.resolve();
}
- events.emit('log', 'Discovered plugin "' + featureId + '" in config.xml. Adding it to the project');
- var pluginEntry = cfg.getPlugin(featureId);
+
+ events.emit('log', `Discovered saved plugin "${pluginName}". Adding it to the project`);
// Install from given URL if defined or using a plugin id. If spec isn't a valid version or version range,
// assume it is the location to install from.
- var pluginSpec = pluginEntry.spec;
- pluginName = pluginEntry.name;
-
// CB-10761 If plugin spec is not specified, use plugin name
- var installFrom = pluginSpec || pluginName;
- if (pluginSpec && semver.validRange(pluginSpec, true)) {
- installFrom = pluginName + '@' + pluginSpec;
+ var installFrom = pluginConfig.spec || pluginName;
+ if (pluginConfig.spec && semver.validRange(pluginConfig.spec, true)) {
+ installFrom = pluginName + '@' + pluginConfig.spec;
}
// Add feature preferences as CLI variables if have any
var options = {
- cli_variables: pluginEntry.variables,
+ cli_variables: pluginConfig.variables,
searchpath: args.searchpath,
save: args.save || false
};
- var plugin = require('./plugin');
+
+ const plugin = require('./plugin');
return plugin('add', installFrom, options);
}, function (error) {
// CB-10921 emit a warning in case of error
diff --git a/src/cordova/util.js b/src/cordova/util.js
index 6246d76..5c1e894 100644
--- a/src/cordova/util.js
+++ b/src/cordova/util.js
@@ -50,6 +50,7 @@ Object.defineProperty(exports, 'libDirectory', {
});
exports.isCordova = isCordova;
+exports.getProjectRoot = getProjectRoot;
exports.cdProjectRoot = cdProjectRoot;
exports.deleteSvnFolders = deleteSvnFolders;
exports.listPlatforms = listPlatforms;
@@ -151,12 +152,24 @@ function isCordova (dir) {
return false;
}
-// Cd to project root dir and return its path. Throw CordovaError if not in a Corodva project.
-function cdProjectRoot () {
- var projectRoot = convertToRealPathSafe(this.isCordova());
+/**
+ * Returns the project root directory path.
+ *
+ * Throws a CordovaError if not in a Cordova project.
+ */
+function getProjectRoot () {
+ const projectRoot = convertToRealPathSafe(this.isCordova());
+
if (!projectRoot) {
throw new CordovaError('Current working directory is not a Cordova-based project.');
}
+
+ return projectRoot;
+}
+
+// Cd to project root dir and return its path. Throw CordovaError if not in a Corodva project.
+function cdProjectRoot () {
+ const projectRoot = this.getProjectRoot();
if (!origCwd) {
origCwd = process.env.PWD || process.cwd();
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org