Skip to content

Commit

Permalink
feat(config): Allow JSON "//" comments in package.json resolutions (y…
Browse files Browse the repository at this point in the history
…arnpkg#4779)

Fixes yarnpkg#4774

**Summary**

Previously package.json comments were being ignored for "dependencies",
"devDependencies", "optionalDependencies".

This change adds "resolutions" to the sections that will ignore
comments.

**Test Plan**

Added unit test to make sure warning is not printed for a comment in a
resolution.
  • Loading branch information
rally25rs authored and BYK committed Oct 26, 2017
1 parent 1ccb710 commit 359b161
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 2 deletions.
24 changes: 24 additions & 0 deletions __tests__/commands/install/integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,30 @@ test.concurrent('install with comments in manifest', (): Promise<void> => {
});
});

test.concurrent('install with comments in manifest resolutions does not result in warning', (): Promise<void> => {
const fixturesLoc = path.join(__dirname, '..', '..', 'fixtures', 'install');

return buildRun(
reporters.BufferReporter,
fixturesLoc,
async (args, flags, config, reporter): Promise<void> => {
await install(config, reporter, flags, args);

const output = reporter.getBuffer();
const warnings = output.filter(entry => entry.type === 'warning');

expect(
warnings.some(warning => {
return warning.data.toString().indexOf(reporter.lang('invalidResolutionName', '//')) > -1;
}),
).toEqual(false);
},
[],
{lockfile: false},
'install-with-comments',
);
});

test.concurrent('install with null versions in manifest', (): Promise<void> => {
return runInstall({}, 'install-with-null-version', async config => {
expect(await fs.exists(path.join(config.cwd, 'node_modules', 'left-pad'))).toEqual(true);
Expand Down
3 changes: 3 additions & 0 deletions __tests__/fixtures/install/install-with-comments/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,8 @@
"comment"
],
"foo": "file:bar"
},
"resolutions": {
"//": "This is a comment."
}
}
2 changes: 2 additions & 0 deletions src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ type Env = {
};

export const DEPENDENCY_TYPES = ['devDependencies', 'dependencies', 'optionalDependencies', 'peerDependencies'];
export const RESOLUTIONS = 'resolutions';
export const MANIFEST_FIELDS = [RESOLUTIONS, ...DEPENDENCY_TYPES];

export const SUPPORTED_NODE_VERSIONS = '^4.8.0 || ^5.7.0 || ^6.2.2 || ^8.0.0';

Expand Down
4 changes: 2 additions & 2 deletions src/util/normalize-manifest/fix.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* @flow */

import {DEPENDENCY_TYPES} from '../../constants';
import {MANIFEST_FIELDS} from '../../constants';
import type {Reporter} from '../../reporters/index.js';
import {isValidLicense} from './util.js';
import {normalizePerson, extractDescription} from './util.js';
Expand Down Expand Up @@ -308,7 +308,7 @@ export default (async function(
}
}

for (const dependencyType of DEPENDENCY_TYPES) {
for (const dependencyType of MANIFEST_FIELDS) {
const dependencyList = info[dependencyType];
if (dependencyList && typeof dependencyList === 'object') {
delete dependencyList['//'];
Expand Down

0 comments on commit 359b161

Please sign in to comment.