Skip to content
Permalink
Browse files
Merge pull request #707 from raphinesse/deprecate-requireCordovaModule
Deprecate `requireCordovaModule` for non-Cordova modules
  • Loading branch information
raphinesse committed Sep 28, 2018
2 parents 3079853 + 74ffd92 commit 81439dc003dacf1ee9c232a0d0d8eba5b23cb18b
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 9 deletions.
@@ -0,0 +1,108 @@
/*!
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
*/

const rewire = require('rewire');
const events = require('cordova-common').events;

describe('hooks/Context', () => {
let Context;

beforeEach(() => {
Context = rewire('../../src/hooks/Context');
});

describe('requireCordovaModule', () => {
let warnSpy, requireCordovaModule;

beforeEach(() => {
requireCordovaModule = Context.prototype.requireCordovaModule;
warnSpy = jasmine.createSpy('warnSpy');
events.on('warn', warnSpy);
});

afterEach(() => {
events.removeListener('warn', warnSpy);
});

it('correctly resolves cordova-* dependencies', () => {
const cordovaCommon = require('cordova-common');
expect(requireCordovaModule('cordova-common')).toBe(cordovaCommon);
});

it('correctly resolves inner modules of cordova-* dependencies', () => {
const MODULE = 'cordova-common/src/events';
expect(requireCordovaModule(MODULE)).toBe(require(MODULE));
});

it('correctly resolves cordova-lib', () => {
const cordovaLib = require('../..');
expect(requireCordovaModule('cordova-lib')).toBe(cordovaLib);
});

it('correctly resolves inner modules of cordova-lib', () => {
const platforms = require('../../src/platforms/platforms');
expect(requireCordovaModule('cordova-lib/src/platforms/platforms')).toBe(platforms);
});

it('correctly resolves inner modules of cordova-lib', () => {
const platforms = require('../../src/platforms/platforms');
expect(requireCordovaModule('cordova-lib/src/platforms/platforms')).toBe(platforms);
});

describe('with stubbed require', () => {
let requireSpy;

beforeEach(() => {
requireSpy = jasmine.createSpy('require');
Context.__set__({ require: requireSpy });
});

it('maps some old paths to their new equivalent', () => {
const ConfigParser = Symbol('ConfigParser');
const xmlHelpers = Symbol('xmlHelpers');
requireSpy.and.returnValue({ ConfigParser, xmlHelpers });

expect(requireCordovaModule('cordova-lib/src/configparser/ConfigParser')).toBe(ConfigParser);
expect(requireCordovaModule('cordova-lib/src/util/xml-helpers')).toBe(xmlHelpers);
expect(requireSpy.calls.allArgs()).toEqual([
['cordova-common'], ['cordova-common']
]);
});

it('correctly handles module names that start with "cordova-lib"', () => {
requireCordovaModule('cordova-libre');
expect(requireSpy).toHaveBeenCalledWith('cordova-libre');
});

it('emits a warning if non-cordova module is requested', () => {
requireCordovaModule('q');

expect(requireSpy).toHaveBeenCalledWith('q');
expect(warnSpy).toHaveBeenCalledTimes(1);

const message = warnSpy.calls.argsFor(0)[0];
expect(message).toContain('requireCordovaModule');
expect(message).toContain('non-cordova module');
expect(message).toContain('deprecated');
expect(message).toContain('"q"');
});
});

});
});
@@ -48,16 +48,33 @@ var compatMap = {
};

/**
* Returns a required module
* @param {String} modulePath Module path
* @returns {Object} */
* Requires the specified Cordova module.
*
* This method should only be used to require packages named `cordova-*`.
* Public modules of such a Cordova module can be required by giving their
* full package path: `cordova-foo/bar` for example.
*
* @param {String} modulePath Module path as specified above
* @returns {*} The required Cordova module
*/
Context.prototype.requireCordovaModule = function (modulePath) {
// There is a very common mistake, when hook requires some cordova functionality
// using 'cordova-lib/...' path.
// This path will be resolved only when running cordova from 'normal' installation
// (without symlinked modules). If cordova-lib linked to cordova-cli this path is
// never resolved, so hook fails with 'Error: Cannot find module 'cordova-lib''
var resolvedPath = path.resolve(__dirname, modulePath.replace(/^cordova-lib/, '../../../cordova-lib'));
const [pkg, ...pkgPath] = modulePath.split('/');

if (!pkg.match(/^cordova-[^/]+/)) {
events.emit('warn',
`Using "requireCordovaModule" to load non-cordova module ` +
`"${modulePath}" is deprecated. Instead, add this module to ` +
`your dependencies and use regular "require" to load it.`
);
}

if (pkg !== 'cordova-lib') return require(modulePath);

// We can only resolve `cordova-lib` by name if this module is installed as
// a dependency of the current main module (e.g. when running `cordova`).
// To handle `cordova-lib` paths correctly in all other cases too, we
// resolve them to real paths before requiring them.
var resolvedPath = path.resolve(__dirname, '../..', ...pkgPath);
var relativePath = path.relative(__dirname, resolvedPath).replace(/\\/g, '/');
events.emit('verbose', 'Resolving module name for ' + modulePath + ' => ' + relativePath);

0 comments on commit 81439dc

Please sign in to comment.