From 86328eaaf900a9dd71cc2f6cc94a5c16df4dbdce Mon Sep 17 00:00:00 2001 From: "Andrey Mikhaylov (lolmaus)" Date: Tue, 26 Sep 2023 11:39:18 +0300 Subject: [PATCH] Throw on incorrect pnpm resolution-mode --- lib/dependency-manager-adapters/pnpm.js | 78 ++----- package.json | 1 + .../pnpm-adapter-test.js | 190 +++++++----------- yarn.lock | 7 + 4 files changed, 104 insertions(+), 172 deletions(-) diff --git a/lib/dependency-manager-adapters/pnpm.js b/lib/dependency-manager-adapters/pnpm.js index e90e3ee8..02207bee 100644 --- a/lib/dependency-manager-adapters/pnpm.js +++ b/lib/dependency-manager-adapters/pnpm.js @@ -12,7 +12,6 @@ const semverGte = require('semver/functions/gte'); const PACKAGE_JSON = 'package.json'; const PNPM_LOCKFILE = 'pnpm-lock.yaml'; -const NPMRC_CONFIG = '.npmrc'; module.exports = CoreObject.extend({ // This still needs to be `npm` because we're still reading the dependencies @@ -26,8 +25,8 @@ module.exports = CoreObject.extend({ }, async setup() { + await this._throwOnResolutionMode(); await this.backup.addFiles([PACKAGE_JSON, PNPM_LOCKFILE]); - await this._updateNpmRc(); }, async changeToDependencySet(depSet) { @@ -54,7 +53,6 @@ module.exports = CoreObject.extend({ await this.backup.restoreFiles([PACKAGE_JSON, PNPM_LOCKFILE]); await this.backup.cleanUp(); await this._install(); - await this._revertNpmRc(); } catch (e) { console.log('Error cleaning up scenario:', e); // eslint-disable-line no-console } @@ -85,72 +83,38 @@ module.exports = CoreObject.extend({ * arguments * @returns Promise */ - async _updateNpmRc(version) { + async _throwOnResolutionMode(version) { if (!version) { version = await this._getPnpmVersion(); } - if (!this._doesPnpmRequireResolutionModeFix(version)) { - return; - } - - let npmrcPath = path.join(this.cwd, NPMRC_CONFIG); - - if (fs.existsSync(npmrcPath)) { - await this.backup.addFile(NPMRC_CONFIG); - - await fs.appendFileSync(npmrcPath, '\nresolution-mode = highest\n'); - } else { - fs.writeFileSync(npmrcPath, 'resolution-mode = highest\n'); + if (await this._isResolutionModeWrong(version)) { + throw new Error( + 'You are using an old version of pnpm that uses wrong resolution mode that violates ember-try expectations. Please either upgrade pnpm or set `resolution-mode` to `highest` in `.npmrc`.' + ); } }, - /** - * pnpm versions 8.0.0 through 8.6.* have the `resolution-mode` setting inverted to - * `lowest-direct`, which breaks ember-try. This method conditionally adds the necessary config to - * .npmrc to fix this. - * - * It covers the following cases: - * - pnpm version is out of dangerious range or cannot be retrieved — do not do anything - * - pnpm version is within dangerous range and the backup does not exist — delete the .npmrc - * - pnpm version is within dangerous range and the backup exists — rename the backup to .npmrc, - * overwriting the current .npmrc - * - * @param {undefined | string} version — this is only used in testing. Call this fucntion without - * arguments - * @returns Promise - */ - async _revertNpmRc(version) { - if (!version) { - version = await this._getPnpmVersion(); - } - - if (!this._doesPnpmRequireResolutionModeFix(version)) { - return; - } - - let npmrcPath = path.join(this.cwd, NPMRC_CONFIG); + async _getPnpmVersion(command = 'pnpm --version') { + let result = await exec(command, { cwd: this.cwd }); + return result.stdout.split('\n')[0]; + }, - if (this.backup.hasFile(NPMRC_CONFIG)) { - await this.backup.restoreFile(NPMRC_CONFIG); - } else { - if (fs.existsSync(npmrcPath)) { - fs.removeSync(npmrcPath); - } - } + async _getResolutionMode() { + let result = await exec('pnpm config get resolution-mode', { cwd: this.cwd }); + return result.stdout.split('\n')[0]; }, - async _getPnpmVersion(command = 'pnpm --version') { - try { - let result = await exec(command); - return result.stdout.split('\n')[0]; - } catch (error) { - return null; + async _isResolutionModeWrong(versionStr = this._getPnpmVersion()) { + if (!versionStr) { + return false; } - }, - _doesPnpmRequireResolutionModeFix(versionStr) { - return versionStr ? semverGte(versionStr, '8.0.0') && semverLt(versionStr, '8.7.0') : false; + return ( + semverGte(versionStr, '8.0.0') && + semverLt(versionStr, '8.7.0') && + (await this._getResolutionMode()) !== 'highest' + ); }, async _install(depSet) { diff --git a/package.json b/package.json index d22a69c8..9d788135 100644 --- a/package.json +++ b/package.json @@ -44,6 +44,7 @@ }, "devDependencies": { "chai": "^4.3.4", + "chai-as-promised": "^7.1.1", "codecov": "^3.8.3", "ember-cli": "~3.22.0", "eslint": "^7.31.0", diff --git a/test/dependency-manager-adapters/pnpm-adapter-test.js b/test/dependency-manager-adapters/pnpm-adapter-test.js index c01c4a46..27e4805f 100644 --- a/test/dependency-manager-adapters/pnpm-adapter-test.js +++ b/test/dependency-manager-adapters/pnpm-adapter-test.js @@ -1,6 +1,9 @@ 'use strict'; -let expect = require('chai').expect; +let chai = require('chai'); +let chaiAsPromised = require('chai-as-promised'); +chai.use(chaiAsPromised); +let { expect } = chai; let fs = require('fs-extra'); let path = require('path'); let tmp = require('tmp-sync'); @@ -12,6 +15,15 @@ let root = process.cwd(); let tmproot = path.join(root, 'tmp'); let tmpdir; +function setResolutionModeToHighest(dir) { + // package.json is required for `pnpm config get` to work + let packageJsonPath = path.join(dir, 'package.json'); + fs.writeFileSync(packageJsonPath, '{"private": true}'); + + let npmrcPath = path.join(dir, '.npmrc'); + fs.writeFileSync(npmrcPath, 'resolution-mode = highest'); +} + describe('pnpm Adapter', () => { beforeEach(() => { tmpdir = tmp.in(tmproot); @@ -25,6 +37,10 @@ describe('pnpm Adapter', () => { }); describe('#setup', () => { + beforeEach(() => { + setResolutionModeToHighest(tmpdir); + }); + it('backs up the `package.json` and `pnpm-lock.yaml` files', async () => { await fs.outputJson('package.json', { originalPackageJSON: true }); await fs.outputFile('pnpm-lock.yaml', 'originalYAML: true\n'); @@ -54,6 +70,10 @@ describe('pnpm Adapter', () => { }); describe('#changeToDependencySet', () => { + beforeEach(() => { + setResolutionModeToHighest(tmpdir); + }); + it('updates the `package.json` and runs `pnpm install`', async () => { await fs.outputJson('package.json', { devDependencies: { @@ -111,7 +131,7 @@ describe('pnpm Adapter', () => { expect(runCount).to.equal(1); }); - it('runs _updateNpmRc before _install', async () => { + it('runs _throwOnResolutionMode before _install', async () => { await fs.outputJson('package.json', { devDependencies: { 'ember-try-test-suite-helper': '0.1.0', @@ -124,7 +144,7 @@ describe('pnpm Adapter', () => { const updateStub = sinon.replace( adapter, - '_updateNpmRc', + '_throwOnResolutionMode', sinon.fake(() => {}) ); @@ -146,6 +166,10 @@ describe('pnpm Adapter', () => { }); describe('#cleanup', () => { + beforeEach(() => { + setResolutionModeToHighest(tmpdir); + }); + it('restores the `package.json` and `pnpm-lock.yaml` files, and then runs `pnpm install`', async () => { await fs.outputJson('package.json', { originalPackageJSON: true }); await fs.outputFile('pnpm-lock.yaml', 'originalYAML: true\n'); @@ -182,33 +206,6 @@ describe('pnpm Adapter', () => { expect(runCount).to.equal(1); }); - - it('runs _revertNpmRc after _install', async () => { - await fs.outputJson('package.json', { modifiedPackageJSON: true }); - await fs.outputJson('package.json.ember-try', { originalPackageJSON: true }); - await fs.outputFile('pnpm-lock.yaml', 'modifiedYAML: true\n'); - await fs.outputFile('pnpm-lock.ember-try.yaml', 'originalYAML: true\n'); - - let adapter = new PnpmAdapter({ - cwd: tmpdir, - }); - - const revertStub = sinon.replace( - adapter, - '_revertNpmRc', - sinon.fake(() => {}) - ); - - const installStub = sinon.replace( - adapter, - '_install', - sinon.fake(() => {}) - ); - - await adapter.cleanup(); - - expect(revertStub.calledAfter(installStub)).to.be.true; - }); }); describe('#_packageJSONForDependencySet', () => { @@ -393,21 +390,35 @@ describe('pnpm Adapter', () => { }); }); - describe('#_doesPnpmRequireResolutionModeFix', () => { + describe('#_isResolutionModeWrong', () => { + // prettier-ignore [ - { given: null, expected: false }, - { given: '1.0.0', expected: false }, - { given: '7.9.9999', expected: false }, - { given: '8.0.0', expected: true }, - { given: '8.1.2', expected: true }, - { given: '8.6.9999', expected: true }, - { given: '8.7.0', expected: false }, - { given: '8.7.1', expected: false }, - { given: '9.0.0', expected: false }, - ].forEach(({ given, expected }) => { - it(`works with given version "${given}"`, () => { + { pnpmVersion: null, npmrc: false, expected: false }, + { pnpmVersion: '1.0.0', npmrc: false, expected: false }, + { pnpmVersion: '7.9.9999', npmrc: false, expected: false }, + { pnpmVersion: '8.0.0', npmrc: false, expected: true }, + { pnpmVersion: '8.1.2', npmrc: false, expected: true }, + { pnpmVersion: '8.6.9999', npmrc: false, expected: true }, + { pnpmVersion: '8.7.0', npmrc: false, expected: false }, + { pnpmVersion: '8.7.1', npmrc: false, expected: false }, + { pnpmVersion: '9.0.0', npmrc: false, expected: false }, + { pnpmVersion: null, npmrc: true, expected: false }, + { pnpmVersion: '1.0.0', npmrc: true, expected: false }, + { pnpmVersion: '7.9.9999', npmrc: true, expected: false }, + { pnpmVersion: '8.0.0', npmrc: true, expected: false }, + { pnpmVersion: '8.1.2', npmrc: true, expected: false }, + { pnpmVersion: '8.6.9999', npmrc: true, expected: false }, + { pnpmVersion: '8.7.0', npmrc: true, expected: false }, + { pnpmVersion: '8.7.1', npmrc: true, expected: false }, + { pnpmVersion: '9.0.0', npmrc: true, expected: false }, + ].forEach(({ pnpmVersion, npmrc, expected }) => { + it(`works with given version ${pnpmVersion} and npmrc ${npmrc ? '' : 'not '}overridden`, async () => { + if (npmrc) { + setResolutionModeToHighest(tmpdir); + } + let npmAdapter = new PnpmAdapter({ cwd: tmpdir }); - let result = npmAdapter._doesPnpmRequireResolutionModeFix(given); + let result = await npmAdapter._isResolutionModeWrong(pnpmVersion); expect(result).equal(expected); }); }); @@ -428,103 +439,52 @@ describe('pnpm Adapter', () => { }); }); - describe('#_updateNpmRc', () => { - describe('when pnpm version requires the resolution-mode fix', () => { - it(`should create a new .npmrc file when none exists`, async () => { - let npmAdapter = new PnpmAdapter({ cwd: tmpdir }); - let npmrcPath = path.join(tmpdir, '.npmrc'); - let npmrcBackupPath = path.join(tmpdir, '.npmrc.ember-try'); - - await npmAdapter._updateNpmRc('8.6.0'); - - let actualFileContent = fs.readFileSync(npmrcPath, 'utf8'); - let expectedFileContent = 'resolution-mode = highest\n'; - - expect(actualFileContent, '.npmrc content').to.equal(expectedFileContent); - expect(fs.existsSync(npmrcBackupPath), '.npmrc.ember-try does not exist').to.be.false; - }); - - it(`should update an npmrc file when it already exists`, async () => { - let npmAdapter = new PnpmAdapter({ cwd: tmpdir }); - let npmrcPath = path.join(tmpdir, '.npmrc'); - let npmrcBackupPath = npmAdapter.backup.pathForFile('.npmrc'); - - fs.writeFileSync(npmrcPath, 'foo = bar\n'); - - await npmAdapter._updateNpmRc('8.6.0'); - - let actualFileContent = fs.readFileSync(npmrcPath, 'utf8'); - let expectedFileContent = 'foo = bar\n\nresolution-mode = highest\n'; - expect(actualFileContent, '.npmrc content').to.equal(expectedFileContent); + describe('#_getResolutionMode', () => { + it('when no .npmrc is present, it should return an empty string', async () => { + let npmAdapter = new PnpmAdapter({ cwd: tmpdir }); - let actualBackupFileContent = fs.readFileSync(npmrcBackupPath, 'utf8'); - let expectedBackupFileContent = 'foo = bar\n'; - expect(actualBackupFileContent, '.npmrc-backup content').to.equal( - expectedBackupFileContent - ); - }); + let result = await npmAdapter._getResolutionMode(); + expect(result).equal(''); }); - describe('when pnpm version does not the resolution-mode fix', () => { - it(`should not create a new .npmrc file`, async () => { - let npmAdapter = new PnpmAdapter({ cwd: tmpdir }); - let npmrcPath = path.join(tmpdir, '.npmrc'); - let npmrcBackupPath = path.join(tmpdir, '.npmrc.ember-try'); + it('when .npmrc contains reslution-mode, it should return the given resolution mode', async () => { + let npmAdapter = new PnpmAdapter({ cwd: tmpdir }); - await npmAdapter._updateNpmRc('7.6.0'); + setResolutionModeToHighest(tmpdir); - expect(fs.existsSync(npmrcPath), '.npmrc does not exist').to.be.false; - expect(fs.existsSync(npmrcBackupPath), '.npmrc.ember-try does not exist').to.be.false; - }); + let result = await npmAdapter._getResolutionMode(); + expect(result).equal('highest'); }); }); - describe('#_revertNpmRc', () => { + describe('#_throwOnResolutionMode', () => { describe('when pnpm version requires the resolution-mode fix', () => { - it(`when backup does not exist, it should delete the .npmrc file`, async () => { + it(`when resoultion-mode is not highest, should throw an error`, async () => { let npmAdapter = new PnpmAdapter({ cwd: tmpdir }); - let npmrcPath = path.join(tmpdir, '.npmrc'); - let npmrcBackupPath = path.join(tmpdir, '.npmrc.ember-try'); - - fs.writeFileSync(npmrcPath, 'resolution-mode = highest\n'); - - await npmAdapter._revertNpmRc('8.6.0'); - expect(fs.existsSync(npmrcPath), '.npmrc.ember-try does not exist').to.be.false; - expect(fs.existsSync(npmrcBackupPath), '.npmrc.ember-try does not exist').to.be.false; + return expect(npmAdapter._throwOnResolutionMode('8.6.0')).to.eventually.be.rejectedWith( + 'You are using an old version of pnpm that uses wrong resolution mode that violates ember-try expectations. Please either upgrade pnpm or set `resolution-mode` to `highest` in `.npmrc`.' + ); }); - it(`when backup exists, it should replace the original file with backup and delete the backup`, async () => { + it(`when resoultion-mode is highest, should not throw an error`, async () => { let npmAdapter = new PnpmAdapter({ cwd: tmpdir }); - let npmrcPath = path.join(tmpdir, '.npmrc'); - let npmrcBackupPath = npmAdapter.backup.pathForFile('.npmrc'); - - fs.writeFileSync(npmrcPath, 'foo = bar\n\nresolution-mode = highest\n'); - fs.writeFileSync(npmrcBackupPath, 'foo = bar\n'); - await npmAdapter._revertNpmRc('8.6.0'); + setResolutionModeToHighest(tmpdir); - let actualFileContent = fs.readFileSync(npmrcPath, 'utf8'); - let expectedFileContent = 'foo = bar\n'; - - expect(actualFileContent, '.npmrc content').to.equal(expectedFileContent); - // expect(fs.existsSync(npmrcBackupPath), '.npmrc.ember-try existence').to.be.false; + await npmAdapter._throwOnResolutionMode('8.6.0'); }); }); describe('when pnpm version does not the resolution-mode fix', () => { - it(`should not touch the existing .npmrc file`, async () => { + it(`should not throw an error`, async () => { let npmAdapter = new PnpmAdapter({ cwd: tmpdir }); let npmrcPath = path.join(tmpdir, '.npmrc'); let npmrcBackupPath = path.join(tmpdir, '.npmrc.ember-try'); - fs.writeFileSync(npmrcPath, 'foo = bar\n'); - - await npmAdapter._revertNpmRc('7.6.0'); + await npmAdapter._throwOnResolutionMode('7.6.0'); - let actualFileContent = fs.readFileSync(npmrcPath, 'utf8'); - - expect(actualFileContent, '.npmrc content').to.equal('foo = bar\n'); + expect(fs.existsSync(npmrcPath), '.npmrc does not exist').to.be.false; expect(fs.existsSync(npmrcBackupPath), '.npmrc.ember-try does not exist').to.be.false; }); }); diff --git a/yarn.lock b/yarn.lock index 7facb2d8..c12a564b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1717,6 +1717,13 @@ cardinal@^1.0.0: ansicolors "~0.2.1" redeyed "~1.0.0" +chai-as-promised@^7.1.1: + version "7.1.1" + resolved "https://registry.yarnpkg.com/chai-as-promised/-/chai-as-promised-7.1.1.tgz#08645d825deb8696ee61725dbf590c012eb00ca0" + integrity sha512-azL6xMoi+uxu6z4rhWQ1jbdUhOMhis2PvscD/xjLqNMkv3BPPp2JyyuTHOrf9BOosGpNQ11v6BKv/g57RXbiaA== + dependencies: + check-error "^1.0.2" + chai@^4.3.4: version "4.3.4" resolved "https://registry.yarnpkg.com/chai/-/chai-4.3.4.tgz#b55e655b31e1eac7099be4c08c21964fce2e6c49"