Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Fix/Support npm file references #281

Merged
merged 5 commits into from
Dec 20, 2017
Merged

Fix/Support npm file references #281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3404001
Rebase file references
Nov 17, 2017
9a50941
Added unit tests for file references
Nov 17, 2017
239b89b
Update README
Nov 17, 2017
697be1e
Do not rebase on absolute file references
Nov 18, 2017
c0c12d2
Fix npm file references package lock (#304)
franciscocpg Dec 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,13 @@ custom:
If you specify a module in both arrays, `forceInclude` and `forceExclude`, the
exclude wins and the module will not be packaged.

#### Local modules

You can use `file:` version references in your `package.json` to use a node module
from a local folder (e.g. `"mymodule": "file:../../myOtherProject/mymodule"`).
With that you can do test deployments from the local machine with different
module versions or modules before they are published officially.

#### Examples

You can find an example setups in the [`examples`][link-examples] folder.
Expand Down
50 changes: 44 additions & 6 deletions lib/packExternalModules.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,41 @@ const childProcess = require('child_process');
const fse = require('fs-extra');
const isBuiltinModule = require('is-builtin-module');

function rebaseFileReferences(pathToPackageRoot, moduleVersion) {
if (/^file:[^/]{2}/.test(moduleVersion)) {
const filePath = _.replace(moduleVersion, /^file:/, '');
return _.replace(`file:${pathToPackageRoot}/${filePath}`, /\\/g, '/');
}

return moduleVersion;
}

function rebasePackageLock(pathToPackageRoot, module) {
if (module.version) {
module.version = rebaseFileReferences(pathToPackageRoot, module.version);
}

if (module.dependencies) {
_.forIn(module.dependencies, moduleDependency => {
rebasePackageLock(pathToPackageRoot, moduleDependency);
});
}
}

/**
* Add the given modules to a package json's dependencies.
*/
function addModulesToPackageJson(externalModules, packageJson) {
function addModulesToPackageJson(externalModules, packageJson, pathToPackageRoot) {
_.forEach(externalModules, externalModule => {
const splitModule = _.split(externalModule, '@');
// If we have a scoped module we have to re-add the @
if (_.startsWith(externalModule, '@')) {
splitModule.splice(0, 1);
splitModule[0] = '@' + splitModule[0];
}
const moduleVersion = _.join(_.tail(splitModule), '@');
let moduleVersion = _.join(_.tail(splitModule), '@');
// We have to rebase file references to the target package.json
moduleVersion = rebaseFileReferences(pathToPackageRoot, moduleVersion);
packageJson.dependencies = packageJson.dependencies || {};
packageJson.dependencies[_.first(splitModule)] = moduleVersion;
});
Expand Down Expand Up @@ -247,7 +270,8 @@ module.exports = {
description: `Packaged externals for ${this.serverless.service.service}`,
private: true
};
addModulesToPackageJson(compositeModules, compositePackage);
const relPath = path.relative(compositeModulePath, path.dirname(packageJsonPath));
addModulesToPackageJson(compositeModules, compositePackage, relPath);
this.serverless.utils.writeFileSync(compositePackageJson, JSON.stringify(compositePackage, null, 2));

// (1.a.2) Copy package-lock.json if it exists, to prevent unwanted upgrades
Expand All @@ -256,8 +280,21 @@ module.exports = {
.then(exists => {
if (exists) {
this.serverless.cli.log('Package lock found - Using locked versions');
return BbPromise.fromCallback(cb => fse.copy(packageLockPath, path.join(compositeModulePath, 'package-lock.json'), cb))
.catch(err => this.serverless.cli.log(`Warning: Could not copy lock file: ${err.message}`));
try {
const packageLockJson = this.serverless.utils.readFileSync(packageLockPath);
/**
* We should not be modifying 'package-lock.json'
* because this file should be treat as internal to npm.
*
* Rebase package-lock is a temporary workaround and must be
* removed as soon as https://github.com/npm/npm/issues/19183 gets fixed.
*/
rebasePackageLock(relPath, packageLockJson);

this.serverless.utils.writeFileSync(path.join(compositeModulePath, 'package-lock.json'), JSON.stringify(packageLockJson, null, 2));
} catch(err) {
this.serverless.cli.log(`Warning: Could not read lock file: ${err.message}`);
}
}
return BbPromise.resolve();
})
Expand Down Expand Up @@ -288,7 +325,8 @@ module.exports = {
_.map(packageForceIncludes, whitelistedPackage => ({ external: whitelistedPackage }))
), packagePath, dependencyGraph);
removeExcludedModules.call(this, prodModules, packageForceExcludes);
addModulesToPackageJson(prodModules, modulePackage);
const relPath = path.relative(modulePath, path.dirname(packageJsonPath));
addModulesToPackageJson(prodModules, modulePackage, relPath);
this.serverless.utils.writeFileSync(modulePackageJson, JSON.stringify(modulePackage, null, 2));

// GOOGLE: Copy modules only if not google-cloud-functions
Expand Down
33 changes: 33 additions & 0 deletions tests/mocks/packageLocalRef.mock.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
{
"dependencies": {
"archiver": "^2.0.0",
"bluebird": "^3.4.0",
"fs-extra": "^0.26.7",
"glob": "^7.1.2",
"localmodule": "file:../../mymodule",
"lodash": "^4.17.4",
"npm-programmatic": "0.0.5",
"uuid": "^5.4.1",
"ts-node": "^3.2.0",
"@scoped/vendor": "1.0.0",
"pg": "^4.3.5"
},
"devDependencies": {
"babel-eslint": "^7.2.3",
"chai": "^4.1.0",
"chai-as-promised": "^7.1.1",
"eslint": "^4.3.0",
"eslint-plugin-import": "^2.7.0",
"eslint-plugin-lodash": "^2.4.4",
"eslint-plugin-promise": "^3.5.0",
"istanbul": "^0.4.5",
"mocha": "^3.4.2",
"mockery": "^2.1.0",
"serverless": "^1.17.0",
"sinon": "^2.3.8",
"sinon-chai": "^2.12.0"
},
"peerDependencies": {
"webpack": "*"
}
}
158 changes: 148 additions & 10 deletions tests/packExternalModules.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const Serverless = require('serverless');
const childProcessMockFactory = require('./mocks/child_process.mock');
const fsExtraMockFactory = require('./mocks/fs-extra.mock');
const packageMock = require('./mocks/package.mock.json');
const packageLocalRefMock = require('./mocks/packageLocalRef.mock.json');

chai.use(require('chai-as-promised'));
chai.use(require('sinon-chai'));
Expand All @@ -29,6 +30,7 @@ describe('packExternalModules', () => {
let fsExtraMock;
// Serverless stubs
let writeFileSyncStub;
let readFileSyncStub;

before(() => {
sandbox = sinon.sandbox.create();
Expand Down Expand Up @@ -59,6 +61,7 @@ describe('packExternalModules', () => {
_.set(serverless, 'service.service', 'test-service');

writeFileSyncStub = sandbox.stub(serverless.utils, 'writeFileSync');
readFileSyncStub = sandbox.stub(serverless.utils, 'readFileSync');
_.set(serverless, 'service.custom.webpackIncludeModules', true);

module = _.assign({
Expand All @@ -72,6 +75,7 @@ describe('packExternalModules', () => {
afterEach(() => {
// Reset all counters and restore all stubbed functions
writeFileSyncStub.reset();
readFileSyncStub.reset();
childProcessMock.exec.reset();
fsExtraMock.pathExists.reset();
fsExtraMock.copy.reset();
Expand Down Expand Up @@ -151,6 +155,50 @@ describe('packExternalModules', () => {
}
]
};
const statsWithFileRef = {
stats: [
{
compilation: {
chunks: [
{
modules: [
{
identifier: _.constant('"crypto"')
},
{
identifier: _.constant('"uuid/v4"')
},
{
identifier: _.constant('external "eslint"')
},
{
identifier: _.constant('"mockery"')
},
{
identifier: _.constant('"@scoped/vendor/module1"')
},
{
identifier: _.constant('external "@scoped/vendor/module2"')
},
{
identifier: _.constant('external "uuid/v4"')
},
{
identifier: _.constant('external "localmodule"')
},
{
identifier: _.constant('external "bluebird"')
},
]
}
],
compiler: {
outputPath: '/my/Service/Path/.webpack/service'
}
}
}
]
};

it('should do nothing if webpackIncludeModules is not set', () => {
_.unset(serverless, 'service.custom.webpackIncludeModules');
Expand Down Expand Up @@ -212,6 +260,102 @@ describe('packExternalModules', () => {
]));
});

it('should rebase file references', () => {
const expectedLocalModule = 'file:../../locals/../../mymodule';
const expectedCompositePackageJSON = {
name: 'test-service',
version: '1.0.0',
description: 'Packaged externals for test-service',
private: true,
dependencies: {
'@scoped/vendor': '1.0.0',
uuid: '^5.4.1',
localmodule: 'file:../../locals/../../mymodule',
bluebird: '^3.4.0'
}
};
const expectedPackageJSON = {
dependencies: {
'@scoped/vendor': '1.0.0',
uuid: '^5.4.1',
localmodule: expectedLocalModule,
bluebird: '^3.4.0'
}
};

const fakePackageLockJSON = {
name: 'test-service',
version: '1.0.0',
description: 'Packaged externals for test-service',
private: true,
dependencies: {
'@scoped/vendor': '1.0.0',
uuid: {
version: '^5.4.1'
},
bluebird: {
version: '^3.4.0'
},
localmodule: {
version: 'file:../../mymodule'
}
}
};
const expectedPackageLockJSON = {
name: 'test-service',
version: '1.0.0',
description: 'Packaged externals for test-service',
private: true,
dependencies: {
'@scoped/vendor': '1.0.0',
uuid: {
version: '^5.4.1'
},
bluebird: {
version: '^3.4.0'
},
localmodule: {
version: expectedLocalModule
}
}
};

_.set(serverless, 'service.custom.webpackIncludeModules.packagePath', path.join('locals', 'package.json'));
module.webpackOutputPath = 'outputPath';
readFileSyncStub.returns(fakePackageLockJSON);
fsExtraMock.pathExists.yields(null, true);
fsExtraMock.copy.yields();
childProcessMock.exec.onFirstCall().yields(null, '{}', '');
childProcessMock.exec.onSecondCall().yields(null, '', '');
childProcessMock.exec.onThirdCall().yields();
module.compileStats = statsWithFileRef;

sandbox.stub(process, 'cwd').returns(path.join('/my/Service/Path'));
mockery.registerMock(path.join(process.cwd(), 'locals', 'package.json'), packageLocalRefMock);

return expect(module.packExternalModules()).to.be.fulfilled
.then(() => BbPromise.all([
// The module package JSON and the composite one should have been stored
expect(writeFileSyncStub).to.have.been.calledThrice,
expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)),
expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageLockJSON, null, 2)),
expect(writeFileSyncStub.thirdCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)),
// The modules should have been copied
expect(fsExtraMock.copy).to.have.been.calledOnce,
// npm ls and npm prune should have been called
expect(childProcessMock.exec).to.have.been.calledThrice,
expect(childProcessMock.exec.firstCall).to.have.been.calledWith(
'npm ls -prod -json -depth=1'
),
expect(childProcessMock.exec.secondCall).to.have.been.calledWith(
'npm install'
),
expect(childProcessMock.exec.thirdCall).to.have.been.calledWith(
'npm prune'
)
]));
});

it('should skip module copy for Google provider', () => {
const expectedCompositePackageJSON = {
name: 'test-service',
Expand Down Expand Up @@ -601,10 +745,7 @@ describe('packExternalModules', () => {
expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)),
expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)),
// The modules should have been copied
expect(fsExtraMock.copy).to.have.been.calledTwice,
expect(fsExtraMock.copy.firstCall).to.have.been.calledWith(
sinon.match(/package-lock.json$/)
),
expect(fsExtraMock.copy).to.have.been.calledOnce,
// npm ls and npm prune should have been called
expect(childProcessMock.exec).to.have.been.calledThrice,
expect(childProcessMock.exec.firstCall).to.have.been.calledWith(
Expand Down Expand Up @@ -640,9 +781,9 @@ describe('packExternalModules', () => {
};

module.webpackOutputPath = 'outputPath';
readFileSyncStub.throws(new Error('Failed to read package-lock.json'));
fsExtraMock.pathExists.yields(null, true);
fsExtraMock.copy.onFirstCall().yields(new Error('Failed to read package-lock.json'));
fsExtraMock.copy.onSecondCall().yields();
fsExtraMock.copy.onFirstCall().yields();
childProcessMock.exec.onFirstCall().yields(null, '{}', '');
childProcessMock.exec.onSecondCall().yields(null, '', '');
childProcessMock.exec.onThirdCall().yields();
Expand All @@ -654,10 +795,7 @@ describe('packExternalModules', () => {
expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)),
expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)),
// The modules should have been copied
expect(fsExtraMock.copy).to.have.been.calledTwice,
expect(fsExtraMock.copy.firstCall).to.have.been.calledWith(
sinon.match(/package-lock.json$/)
),
expect(fsExtraMock.copy).to.have.been.calledOnce,
// npm ls and npm prune should have been called
expect(childProcessMock.exec).to.have.been.calledThrice,
expect(childProcessMock.exec.firstCall).to.have.been.calledWith(
Expand Down