Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-706 dependency patch fix for 8.x only #708

Merged
merged 8 commits into from Sep 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions package.json
Expand Up @@ -24,6 +24,7 @@
"cordova-js": "^4.2.2",
"cordova-serve": "^2.0.0",
"dep-graph": "1.1.0",
"dependency-ls": "^1.1.1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this and all the other dependencies that are readded are not reflected in the PR title and description

"detect-indent": "^5.0.0",
"elementtree": "^0.1.7",
"glob": "^7.1.2",
Expand All @@ -34,12 +35,16 @@
"properties-parser": "0.3.1",
"q": "^1.5.1",
"read-chunk": "^2.1.0",
"request": "^2.88.0",
"semver": "^5.3.0",
"shebang-command": "^1.2.0",
"shelljs": "0.3.0",
"tar": "^4.4.1",
"tar": "^2.2.1",
"underscore": "^1.9.0",
"which": "^1.3.1"
"unorm": "^1.4.1",
"valid-identifier": "0.0.1",
"which": "^1.3.1",
"xcode": "^1.0.0"
},
"devDependencies": {
"codecov": "^3.0.1",
Expand Down
97 changes: 97 additions & 0 deletions spec/hooks/Context.spec.js
@@ -0,0 +1,97 @@
/*!
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.
*/

'use strict';

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');
});
});

});
});
27 changes: 19 additions & 8 deletions src/hooks/Context.js
Expand Up @@ -17,6 +17,8 @@
under the License.
*/

'use strict';

var path = require('path');
var events = require('cordova-common').events;

Expand Down Expand Up @@ -54,15 +56,24 @@ 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''
const pkg = modulePath.split('/')[0];

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, modulePath.replace(/^cordova-lib/, '../../../cordova-lib'));
var relativePath = path.relative(__dirname, resolvedPath).replace(/\\/g, '/');
events.emit('verbose', 'Resolving module name for ' + modulePath + ' => ' + relativePath);
Expand Down