Skip to content

Commit

Permalink
Merge pull request #1252 from jagonzalr/detect-yarn-2-or-above
Browse files Browse the repository at this point in the history 
Detect yarn version to dynamically not add deprecated flags
  • Loading branch information
@j0k3r
j0k3r committed Oct 14, 2022
2 parents 035f40f + eb892ec commit ffefe00
Show file tree
Hide file tree
Showing 7 changed files with 151 additions and 30 deletions.
41 changes: 23 additions & 18 deletions lib/packExternalModules.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -397,17 +397,20 @@ module.exports = {
} else {
this.serverless.cli.log('Packing external modules: ' + compositeModules.join(', '));
}
return packager
.install(compositeModulePath, this.configuration.packagerOptions)
.then(() => {
if (this.log) {
this.log.verbose(`Package took [${_.now() - start} ms]`);
} else {
this.options.verbose && this.serverless.cli.log(`Package took [${_.now() - start} ms]`);
}
return null;
})
.return(stats.stats);

return packager.getPackagerVersion(compositeModulePath).then(version => {
return packager
.install(compositeModulePath, this.configuration.packagerOptions, version)
.then(() => {
if (this.log) {
this.log.verbose(`Package took [${_.now() - start} ms]`);
} else {
this.options.verbose && this.serverless.cli.log(`Package took [${_.now() - start} ms]`);
}
return null;
})
.return(stats.stats);
});
})
.mapSeries(compileStats => {
const modulePath = compileStats.outputPath;
Expand Down Expand Up @@ -485,13 +488,15 @@ module.exports = {
.then(() => {
// Prune extraneous packages - removes not needed ones
const startPrune = _.now();
return packager.prune(modulePath, this.configuration.packagerOptions).tap(() => {
if (this.log) {
this.log.verbose(`Prune: ${modulePath} [${_.now() - startPrune} ms]`);
} else {
this.options.verbose &&
this.serverless.cli.log(`Prune: ${modulePath} [${_.now() - startPrune} ms]`);
}
return packager.getPackagerVersion(modulePath).then(version => {
return packager.prune(modulePath, this.configuration.packagerOptions, version).tap(() => {
if (this.log) {
this.log.verbose(`Prune: ${modulePath} [${_.now() - startPrune} ms]`);
} else {
this.options.verbose &&
this.serverless.cli.log(`Prune: ${modulePath} [${_.now() - startPrune} ms]`);
}
});
});
})
.then(() => {
Expand Down
1 change: 1 addition & 0 deletions lib/packagers/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* static get lockfileName(): string;
* static get copyPackageSectionNames(): Array<string>;
* static get mustCopyModules(): boolean;
* static getPackagerVersion(cwd: string): BbPromise<Object>
* 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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,17 @@ class NPM {
return true;
}

static getPackagerVersion(cwd) {
const command = /^win/.test(process.platform) ? 'npm.cmd' : 'npm';
const args = ['-v'];

return Utils.spawnProcess(command, args, { cwd })
.catch(err => {
return BbPromise.resolve({ stdout: err.stdout });
})
.then(processOutput => processOutput.stdout);
}

static getProdDependencies(cwd, depth, packagerOptions) {
// Try to use NPM lockfile v2 when possible
const options = packagerOptions || {};
Expand Down
33 changes: 27 additions & 6 deletions lib/packagers/yarn.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,23 @@ class Yarn {
return false;
}

static isBerryVersion(version) {
const versionNumber = version.charAt(0);
const mainVersion = parseInt(versionNumber);
return mainVersion > 1;
}

static getPackagerVersion(cwd) {
const command = /^win/.test(process.platform) ? 'yarn.cmd' : 'yarn';
const args = ['-v'];

return Utils.spawnProcess(command, args, { cwd })
.catch(err => {
return BbPromise.resolve({ stdout: err.stdout });
})
.then(processOutput => processOutput.stdout);
}

static getProdDependencies(cwd, depth) {
const command = /^win/.test(process.platform) ? 'yarn.cmd' : 'yarn';
const args = ['list', `--depth=${depth || 1}`, '--json', '--production'];
Expand Down Expand Up @@ -118,20 +135,24 @@ class Yarn {
return _.reduce(replacements, (__, replacement) => _.replace(__, replacement.oldRef, replacement.newRef), lockfile);
}

static install(cwd, packagerOptions) {
static install(cwd, packagerOptions, version) {
if (packagerOptions.noInstall) {
return BbPromise.resolve();
}
const isBerry = Yarn.isBerryVersion(version);

const command = /^win/.test(process.platform) ? 'yarn.cmd' : 'yarn';
const args = ['install'];

// Convert supported packagerOptions
if (!packagerOptions.noNonInteractive) {
if (!packagerOptions.noNonInteractive && !isBerry) {
args.push('--non-interactive');
}
if (!packagerOptions.noFrozenLockfile) {
args.push('--frozen-lockfile');
if (isBerry) {
args.push('--immutable');
} else {
args.push('--frozen-lockfile');
}
}
if (packagerOptions.ignoreScripts) {
args.push('--ignore-scripts');
Expand All @@ -144,8 +165,8 @@ class Yarn {
}

// "Yarn install" prunes automatically
static prune(cwd, packagerOptions) {
return Yarn.install(cwd, packagerOptions);
static prune(cwd, packagerOptions, version) {
return Yarn.install(cwd, packagerOptions, version);
}

static runScripts(cwd, scriptNames) {
Expand Down
11 changes: 11 additions & 0 deletions tests/packExternalModules.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ jest.mock('../lib/packagers/index', () => {
copyPackageSectionNames: ['section1', 'section2'],
mustCopyModules: true,
rebaseLockfile: jest.fn(),
getPackagerVersion: jest.fn(),
getProdDependencies: jest.fn(),
install: jest.fn(),
prune: jest.fn(),
Expand Down Expand Up @@ -203,6 +204,7 @@ describe('packExternalModules', () => {
fsExtraMock.pathExists.mockImplementation((p, cb) => cb(null, false));
fsExtraMock.copy.mockImplementation((from, to, cb) => cb());
packagerFactoryMock.get('npm').getProdDependencies.mockReturnValue(BbPromise.resolve({}));
packagerFactoryMock.get('npm').getPackagerVersion.mockReturnValue(BbPromise.resolve());
packagerFactoryMock.get('npm').install.mockReturnValue(BbPromise.resolve());
packagerFactoryMock.get('npm').prune.mockReturnValue(BbPromise.resolve());
packagerFactoryMock.get('npm').runScripts.mockReturnValue(BbPromise.resolve());
Expand All @@ -219,6 +221,7 @@ describe('packExternalModules', () => {
expect(fsExtraMock.copy).toHaveBeenCalledTimes(1),
// npm ls and npm prune should have been called
expect(packagerFactoryMock.get('npm').getProdDependencies).toHaveBeenCalledTimes(1),
expect(packagerFactoryMock.get('npm').getPackagerVersion).toHaveBeenCalledTimes(2),
expect(packagerFactoryMock.get('npm').install).toHaveBeenCalledTimes(1),
expect(packagerFactoryMock.get('npm').prune).toHaveBeenCalledTimes(1),
expect(packagerFactoryMock.get('npm').runScripts).toHaveBeenCalledTimes(1)
Expand Down Expand Up @@ -256,6 +259,7 @@ describe('packExternalModules', () => {
fsExtraMock.pathExists.mockImplementation((p, cb) => cb(null, false));
fsExtraMock.copy.mockImplementation((from, to, cb) => cb());
packagerFactoryMock.get('npm').getProdDependencies.mockReturnValue(BbPromise.resolve({}));
packagerFactoryMock.get('npm').getPackagerVersion.mockReturnValue(BbPromise.resolve());
packagerFactoryMock.get('npm').install.mockReturnValue(BbPromise.resolve());
packagerFactoryMock.get('npm').prune.mockReturnValue(BbPromise.resolve());
packagerFactoryMock.get('npm').runScripts.mockReturnValue(BbPromise.resolve());
Expand All @@ -272,6 +276,7 @@ describe('packExternalModules', () => {
expect(fsExtraMock.copy).toHaveBeenCalledTimes(1),
// npm ls and npm prune should have been called
expect(packagerFactoryMock.get('npm').getProdDependencies).toHaveBeenCalledTimes(1),
expect(packagerFactoryMock.get('npm').getPackagerVersion).toHaveBeenCalledTimes(2),
expect(packagerFactoryMock.get('npm').install).toHaveBeenCalledTimes(1),
expect(packagerFactoryMock.get('npm').prune).toHaveBeenCalledTimes(1),
expect(packagerFactoryMock.get('npm').runScripts).toHaveBeenCalledTimes(1)
Expand Down Expand Up @@ -341,6 +346,7 @@ describe('packExternalModules', () => {
fsExtraMock.copy.mockImplementation((from, to, cb) => cb());
packagerFactoryMock.get('npm').getProdDependencies.mockReturnValue(BbPromise.resolve({}));
packagerFactoryMock.get('npm').rebaseLockfile.mockImplementation((pathToPackageRoot, lockfile) => lockfile);
packagerFactoryMock.get('npm').getPackagerVersion.mockReturnValue(BbPromise.resolve());
packagerFactoryMock.get('npm').install.mockReturnValue(BbPromise.resolve());
packagerFactoryMock.get('npm').prune.mockReturnValue(BbPromise.resolve());
packagerFactoryMock.get('npm').runScripts.mockReturnValue(BbPromise.resolve());
Expand Down Expand Up @@ -371,6 +377,7 @@ describe('packExternalModules', () => {
),
// npm ls and npm prune should have been called
expect(packagerFactoryMock.get('npm').getProdDependencies).toHaveBeenCalledTimes(1),
expect(packagerFactoryMock.get('npm').getPackagerVersion).toHaveBeenCalledTimes(2),
expect(packagerFactoryMock.get('npm').install).toHaveBeenCalledTimes(1),
expect(packagerFactoryMock.get('npm').prune).toHaveBeenCalledTimes(1),
expect(packagerFactoryMock.get('npm').runScripts).toHaveBeenCalledTimes(1)
Expand Down Expand Up @@ -412,6 +419,7 @@ describe('packExternalModules', () => {
fsExtraMock.pathExists.mockImplementation((p, cb) => cb(null, false));
fsExtraMock.copy.mockImplementation((from, to, cb) => cb());
packagerFactoryMock.get('npm').getProdDependencies.mockReturnValue(BbPromise.resolve({}));
packagerFactoryMock.get('npm').getPackagerVersion.mockReturnValue(BbPromise.resolve());
packagerFactoryMock.get('npm').install.mockReturnValue(BbPromise.resolve());
packagerFactoryMock.get('npm').prune.mockReturnValue(BbPromise.resolve());
packagerFactoryMock.get('npm').runScripts.mockReturnValue(BbPromise.resolve());
Expand All @@ -428,6 +436,7 @@ describe('packExternalModules', () => {
expect(fsExtraMock.copy).toHaveBeenCalledTimes(0),
// npm ls and npm prune should have been called
expect(packagerFactoryMock.get('npm').getProdDependencies).toHaveBeenCalledTimes(1),
expect(packagerFactoryMock.get('npm').getPackagerVersion).toHaveBeenCalledTimes(1),
expect(packagerFactoryMock.get('npm').install).toHaveBeenCalledTimes(1),
expect(packagerFactoryMock.get('npm').prune).toHaveBeenCalledTimes(0),
expect(packagerFactoryMock.get('npm').runScripts).toHaveBeenCalledTimes(0)
Expand All @@ -440,6 +449,7 @@ describe('packExternalModules', () => {
fsExtraMock.pathExists.mockImplementation((p, cb) => cb(null, false));
fsExtraMock.copy.mockImplementation((from, to, cb) => cb());
packagerFactoryMock.get('npm').getProdDependencies.mockReturnValue(BbPromise.resolve({}));
packagerFactoryMock.get('npm').getPackagerVersion.mockReturnValue(BbPromise.resolve());
packagerFactoryMock
.get('npm')
.install.mockImplementation(() => BbPromise.reject(new Error('npm install failed')));
Expand All @@ -452,6 +462,7 @@ describe('packExternalModules', () => {
BbPromise.all([
// npm ls and npm install should have been called
expect(packagerFactoryMock.get('npm').getProdDependencies).toHaveBeenCalledTimes(1),
expect(packagerFactoryMock.get('npm').getPackagerVersion).toHaveBeenCalledTimes(1),
expect(packagerFactoryMock.get('npm').install).toHaveBeenCalledTimes(1),
expect(packagerFactoryMock.get('npm').prune).toHaveBeenCalledTimes(0),
expect(packagerFactoryMock.get('npm').runScripts).toHaveBeenCalledTimes(0)
Expand Down
30 changes: 30 additions & 0 deletions tests/packagers/npm.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,36 @@ describe('npm', () => {
expect(npmModule.mustCopyModules).toBe(true);
});

describe('getPackagerVersion', () => {
it('should use npm version 6.14.17', () => {
const npmVersion = '6.14.17';
Utils.spawnProcess.mockReturnValue(BbPromise.resolve({ stdout: npmVersion, stderr: '' }));
return expect(npmModule.getPackagerVersion('myPath', 1))
.resolves.toEqual(npmVersion)
.then(() => {
expect(Utils.spawnProcess).toHaveBeenCalledTimes(1);
expect(Utils.spawnProcess).toHaveBeenNthCalledWith(1, expect.stringMatching(/^npm/), ['-v'], {
cwd: 'myPath'
});
return null;
});
});

it('should use npm version 8.19.2', () => {
const npmVersion = '8.19.2';
Utils.spawnProcess.mockReturnValue(BbPromise.resolve({ stdout: npmVersion, stderr: '' }));
return expect(npmModule.getPackagerVersion('myPath', 1))
.resolves.toEqual(npmVersion)
.then(() => {
expect(Utils.spawnProcess).toHaveBeenCalledTimes(1);
expect(Utils.spawnProcess).toHaveBeenNthCalledWith(1, expect.stringMatching(/^npm/), ['-v'], {
cwd: 'myPath'
});
return null;
});
});
});

describe('install', () => {
it('should use npm install', () => {
Utils.spawnProcess.mockReturnValue(BbPromise.resolve({ stdout: 'installed successfully', stderr: '' }));
Expand Down
54 changes: 48 additions & 6 deletions tests/packagers/yarn.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,48 @@ describe('yarn', () => {
expect(yarnModule.mustCopyModules).toBe(false);
});

describe('isBerryVersion', () => {
it('Yarn version 1.22.19', () => {
const yarnVersion = '1.22.19';
expect(yarnModule.isBerryVersion(yarnVersion)).toBe(false);
});

it('Yarn version 3.2.3', () => {
const yarnVersion = '3.2.3';
expect(yarnModule.isBerryVersion(yarnVersion)).toBe(true);
});
});

describe('getPackagerVersion', () => {
it('should use yarn version 1.22.19', () => {
const yarnVersion = '1.22.19';
Utils.spawnProcess.mockReturnValue(BbPromise.resolve({ stdout: yarnVersion, stderr: '' }));
return expect(yarnModule.getPackagerVersion('myPath', 1))
.resolves.toEqual(yarnVersion)
.then(() => {
expect(Utils.spawnProcess).toHaveBeenCalledTimes(1);
expect(Utils.spawnProcess).toHaveBeenNthCalledWith(1, expect.stringMatching(/^yarn/), ['-v'], {
cwd: 'myPath'
});
return null;
});
});

it('should use yarn version 3.2.3', () => {
const yarnVersion = '3.2.3';
Utils.spawnProcess.mockReturnValue(BbPromise.resolve({ stdout: yarnVersion, stderr: '' }));
return expect(yarnModule.getPackagerVersion('myPath', 1))
.resolves.toEqual(yarnVersion)
.then(() => {
expect(Utils.spawnProcess).toHaveBeenCalledTimes(1);
expect(Utils.spawnProcess).toHaveBeenNthCalledWith(1, expect.stringMatching(/^yarn/), ['-v'], {
cwd: 'myPath'
});
return null;
});
});
});

describe('getProdDependencies', () => {
it('should use yarn list', () => {
Utils.spawnProcess.mockReturnValue(BbPromise.resolve({ stdout: '{}', stderr: '' }));
Expand Down Expand Up @@ -198,7 +240,7 @@ describe('yarn', () => {
describe('install', () => {
it('should use yarn install', () => {
Utils.spawnProcess.mockReturnValue(BbPromise.resolve({ stdout: 'installed successfully', stderr: '' }));
return expect(yarnModule.install('myPath', {}))
return expect(yarnModule.install('myPath', {}, '1.22.19'))
.resolves.toBeUndefined()
.then(() => {
expect(Utils.spawnProcess).toHaveBeenCalledTimes(1);
Expand All @@ -215,7 +257,7 @@ describe('yarn', () => {

it('should use noNonInteractive option', () => {
Utils.spawnProcess.mockReturnValue(BbPromise.resolve({ stdout: 'installed successfully', stderr: '' }));
return expect(yarnModule.install('myPath', { noNonInteractive: true }))
return expect(yarnModule.install('myPath', { noNonInteractive: true }, '1.22.19'))
.resolves.toBeUndefined()
.then(() => {
expect(Utils.spawnProcess).toHaveBeenCalledTimes(1);
Expand All @@ -232,7 +274,7 @@ describe('yarn', () => {

it('should use ignoreScripts option', () => {
Utils.spawnProcess.mockReturnValue(BbPromise.resolve({ stdout: 'installed successfully', stderr: '' }));
return expect(yarnModule.install('myPath', { ignoreScripts: true }))
return expect(yarnModule.install('myPath', { ignoreScripts: true }, '1.22.19'))
.resolves.toBeUndefined()
.then(() => {
expect(Utils.spawnProcess).toHaveBeenCalledTimes(1);
Expand All @@ -249,7 +291,7 @@ describe('yarn', () => {

it('should use noFrozenLockfile option', () => {
Utils.spawnProcess.mockReturnValue(BbPromise.resolve({ stdout: 'installed successfully', stderr: '' }));
return expect(yarnModule.install('myPath', { noFrozenLockfile: true }))
return expect(yarnModule.install('myPath', { noFrozenLockfile: true }, '1.22.19'))
.resolves.toBeUndefined()
.then(() => {
expect(Utils.spawnProcess).toHaveBeenCalledTimes(1);
Expand All @@ -266,7 +308,7 @@ describe('yarn', () => {

it('should use networkConcurrency option', () => {
Utils.spawnProcess.mockReturnValue(BbPromise.resolve({ stdout: 'installed successfully', stderr: '' }));
return expect(yarnModule.install('myPath', { networkConcurrency: 1 }))
return expect(yarnModule.install('myPath', { networkConcurrency: 1 }, '1.22.19'))
.resolves.toBeUndefined()
.then(() => {
expect(Utils.spawnProcess).toHaveBeenCalledTimes(1);
Expand All @@ -291,7 +333,7 @@ describe('yarn', () => {
describe('prune', () => {
it('should call install', () => {
Utils.spawnProcess.mockReturnValue(BbPromise.resolve({ stdout: 'success', stderr: '' }));
return expect(yarnModule.prune('myPath', {}))
return expect(yarnModule.prune('myPath', {}, '1.22.19'))
.resolves.toBeUndefined()
.then(() => {
expect(Utils.spawnProcess).toHaveBeenCalledTimes(1);
Expand Down

0 comments on commit ffefe00

Please sign in to comment.