Skip to content

Commit

Permalink
Merge pull request #381 from serverless-heaven/use-package-json-sections
Browse files Browse the repository at this point in the history
Use package json sections defined by packagers
  • Loading branch information
HyperBrain committed Apr 30, 2018
2 parents 8549310 + 5793736 commit e1bbace
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 11 deletions.
23 changes: 12 additions & 11 deletions lib/packExternalModules.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,14 @@ module.exports = {
// Determine and create packager
return BbPromise.try(() => Packagers.get.call(this, this.configuration.packager))
.then(packager => {
// Fetch needed original package.json sections
const sectionNames = packager.copyPackageSectionNames;
const packageJson = this.serverless.utils.readFileSync(packageJsonPath);
const packageSections = _.pick(packageJson, sectionNames);
if (!_.isEmpty(packageSections)) {
this.options.verbose && this.serverless.cli.log(`Using package.json sections ${_.join(_.keys(packageSections), ', ')}`);
}

// Get first level dependency graph
this.options.verbose && this.serverless.cli.log(`Fetch dependency graph from ${packageJsonPath}`);

Expand Down Expand Up @@ -227,13 +235,13 @@ module.exports = {
const compositePackageJson = path.join(compositeModulePath, 'package.json');

// (1.a.1) Create a package.json
const compositePackage = {
const compositePackage = _.defaults({
name: this.serverless.service.service,
version: '1.0.0',
description: `Packaged externals for ${this.serverless.service.service}`,
private: true,
scripts: packageScripts
};
}, packageSections);
const relPath = path.relative(compositeModulePath, path.dirname(packageJsonPath));
addModulesToPackageJson(compositeModules, compositePackage, relPath);
this.serverless.utils.writeFileSync(compositePackageJson, JSON.stringify(compositePackage, null, 2));
Expand All @@ -247,13 +255,6 @@ module.exports = {
this.serverless.cli.log('Package lock found - Using locked versions');
try {
let packageLockFile = 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.
*/
packageLockFile = packager.rebaseLockfile(relPath, packageLockFile);
if (_.isObject(packageLockFile)) {
packageLockFile = JSON.stringify(packageLockFile, null, 2);
Expand All @@ -279,14 +280,14 @@ module.exports = {

// Create package.json
const modulePackageJson = path.join(modulePath, 'package.json');
const modulePackage = {
const modulePackage = _.defaults({
name: this.serverless.service.service,
version: '1.0.0',
description: `Packaged externals for ${this.serverless.service.service}`,
private: true,
scripts: packageScripts,
dependencies: {}
};
}, packageSections);
const prodModules = getProdModules.call(this,
_.concat(
getExternalModules.call(this, compileStats),
Expand Down
2 changes: 2 additions & 0 deletions lib/packagers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
* interface Packager {
*
* static get lockfileName(): string;
* static get copyPackageSectionNames(): Array<string>;
* static get mustCopyModules(): boolean;
* static getProdDependencies(cwd: string, depth: number = 1): BbPromise<Object>;
* static rebaseLockfile(pathToPackageRoot: string, lockfile: Object): void;
* static install(cwd: string): BbPromise<void>;
Expand Down
11 changes: 11 additions & 0 deletions lib/packagers/npm.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ class NPM {
return 'package-lock.json';
}

static get copyPackageSectionNames() {
return [];
}

static get mustCopyModules() { // eslint-disable-line lodash/prefer-constant
return true;
}
Expand Down Expand Up @@ -66,6 +70,13 @@ class NPM {
return moduleVersion;
}

/**
* We should not be modifying 'package-lock.json'
* because this file should be treated 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.
*/
static rebaseLockfile(pathToPackageRoot, lockfile) {
if (lockfile.version) {
lockfile.version = NPM._rebaseFileReferences(pathToPackageRoot, lockfile.version);
Expand Down
4 changes: 4 additions & 0 deletions lib/packagers/npm.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ describe('npm', () => {
expect(npmModule.lockfileName).to.equal('package-lock.json');
});

it('should return no packager sections', () => {
expect(npmModule.copyPackageSectionNames).to.be.an('array').that.is.empty;
});

it('requires to copy modules', () => {
expect(npmModule.mustCopyModules).to.be.true;
});
Expand Down
4 changes: 4 additions & 0 deletions lib/packagers/yarn.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ class Yarn {
return 'yarn.lock';
}

static get copyPackageSectionNames() {
return ['resolutions'];
}

static get mustCopyModules() { // eslint-disable-line lodash/prefer-constant
return false;
}
Expand Down
4 changes: 4 additions & 0 deletions lib/packagers/yarn.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ describe('yarn', () => {
expect(yarnModule.lockfileName).to.equal('yarn.lock');
});

it('should return packager sections', () => {
expect(yarnModule.copyPackageSectionNames).to.deep.equal(['resolutions']);
});

it('does not require to copy modules', () => {
expect(yarnModule.mustCopyModules).to.be.false;
});
Expand Down
73 changes: 73 additions & 0 deletions tests/packExternalModules.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const packagerMockFactory = {
create(sandbox) {
const packagerMock = {
lockfileName: 'mocked-lock.json',
copyPackageSectionNames: [ 'section1', 'section2' ],
mustCopyModules: true,
rebaseLockfile: sandbox.stub(),
getProdDependencies: sandbox.stub(),
Expand Down Expand Up @@ -243,6 +244,75 @@ describe('packExternalModules', () => {
]));
});

it('should copy needed package sections if available', () => {
const originalPackageJSON = {
name: 'test-service',
version: '1.0.0',
description: 'Packaged externals for test-service',
private: true,
scripts: {},
section1: {
value: 'myValue'
},
dependencies: {
'@scoped/vendor': '1.0.0',
uuid: '^5.4.1',
bluebird: '^3.4.0'
}
};
const expectedCompositePackageJSON = {
name: 'test-service',
version: '1.0.0',
description: 'Packaged externals for test-service',
private: true,
scripts: {},
section1: originalPackageJSON.section1,
dependencies: {
'@scoped/vendor': '1.0.0',
uuid: '^5.4.1',
bluebird: '^3.4.0'
}
};
const expectedPackageJSON = {
name: 'test-service',
version: '1.0.0',
description: 'Packaged externals for test-service',
private: true,
scripts: {},
dependencies: {
'@scoped/vendor': '1.0.0',
uuid: '^5.4.1',
bluebird: '^3.4.0'
},
section1: originalPackageJSON.section1
};

module.webpackOutputPath = 'outputPath';
readFileSyncStub.onFirstCall().returns(originalPackageJSON);
readFileSyncStub.throws(new Error('Unexpected call to readFileSync'));
fsExtraMock.pathExists.yields(null, false);
fsExtraMock.copy.yields();
packagerMock.getProdDependencies.returns(BbPromise.resolve({}));
packagerMock.install.returns(BbPromise.resolve());
packagerMock.prune.returns(BbPromise.resolve());
packagerMock.runScripts.returns(BbPromise.resolve());
module.compileStats = stats;
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.calledTwice,
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.calledOnce,
// npm ls and npm prune should have been called
expect(packagerMock.getProdDependencies).to.have.been.calledOnce,
expect(packagerMock.install).to.have.been.calledOnce,
expect(packagerMock.prune).to.have.been.calledOnce,
expect(packagerMock.runScripts).to.have.been.calledOnce,
]));
});

it('should install external modules', () => {
const expectedCompositePackageJSON = {
name: 'test-service',
Expand Down Expand Up @@ -349,6 +419,7 @@ describe('packExternalModules', () => {
}
});
module.webpackOutputPath = 'outputPath';
readFileSyncStub.onFirstCall().returns(packageLocalRefMock);
readFileSyncStub.returns(fakePackageLockJSON);
fsExtraMock.pathExists.yields(null, true);
fsExtraMock.copy.yields();
Expand Down Expand Up @@ -718,6 +789,7 @@ describe('packExternalModules', () => {
module.webpackOutputPath = 'outputPath';
fsExtraMock.pathExists.yields(null, true);
fsExtraMock.copy.yields();
readFileSyncStub.onFirstCall().returns(packageMock);
readFileSyncStub.returns({ info: 'lockfile' });
packagerMock.rebaseLockfile.callsFake((pathToPackageRoot, lockfile) => lockfile);
packagerMock.getProdDependencies.returns(BbPromise.resolve({}));
Expand Down Expand Up @@ -769,6 +841,7 @@ describe('packExternalModules', () => {
};

module.webpackOutputPath = 'outputPath';
readFileSyncStub.onFirstCall().returns(packageMock);
readFileSyncStub.throws(new Error('Failed to read package-lock.json'));
fsExtraMock.pathExists.yields(null, true);
fsExtraMock.copy.onFirstCall().yields();
Expand Down

0 comments on commit e1bbace

Please sign in to comment.