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.

Sign up for GitHub

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

Detect yarn version to dynamically not add deprecated flags #1252

Merged
merged 1 commit into from
Oct 14, 2022
Merged

Detect yarn version to dynamically not add deprecated flags #1252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
41 changes: 23 additions & 18 deletions lib/packExternalModules.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
14 changes: 7 additions & 7 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
"eslint-plugin-import": "^2.26.0",
"eslint-plugin-lodash": "^7.4.0",
"eslint-plugin-prettier": "^4.2.1",
"eslint-plugin-promise": "^6.0.1",
"eslint-plugin-promise": "^6.1.0",
"husky": "^4.3.8",
"jest": "^27.5.1",
"lint-staged": "^10.5.4",
Expand Down
11 changes: 11 additions & 0 deletions tests/packExternalModules.test.js
View file
Open in desktop
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
Open in desktop
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
Loading