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.

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

feat(publish): Publish command uses publishConfig.access in package.json #5290

Merged
merged 6 commits into from
Jan 30, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
10 changes: 10 additions & 0 deletions __tests__/__mocks__/npm-registry.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/* @flow */

import Registry from '../../src/registries/base-registry.js';
import type {RegistryRequestOptions} from '../../src/registries/base-registry.js';

export default class NpmRegistry extends Registry {
request(pathname: string, opts?: RegistryRequestOptions = {}, packageName: ?string): Promise<*> {
return new Promise(resolve => resolve());
}
}
71 changes: 71 additions & 0 deletions __tests__/commands/publish.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/* @flow */

import {run as buildRun} from './_helpers.js';
import {run as publish} from '../../src/cli/commands/publish.js';
import {ConsoleReporter} from '../../src/reporters/index.js';

const path = require('path');

const fixturesLoc = path.join(__dirname, '..', 'fixtures', 'publish');

const setupMocks = function(config) {
// Mock actual network request so that no package is actually published.
// $FlowFixMe
config.registries.npm.request = jest.fn();
config.registries.npm.request.mockReturnValue(
new Promise(resolve => {
resolve({status: 200});
}),
);

// Mock the npm login name. Otherwise yarn will prompt for it and break CI.
// $FlowFixMe
config.registries.npm.getAuth = jest.fn();
config.registries.npm.getAuth.mockReturnValue('test');
};

const runPublish = buildRun.bind(
null,
ConsoleReporter,
fixturesLoc,
async (args, flags, config, reporter, lockfile, getStdout): Promise<string> => {
setupMocks(config);
await publish(config, reporter, flags, args);
return getStdout();
},
);

test.concurrent('publish should default access to undefined', () => {
return runPublish([], {newVersion: '0.0.1'}, 'minimal', config => {
const requestCallParams = config.registries.npm.request.mock.calls[0][1];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it more readable to use toBeCalledWith rather than reaching into the mock objects

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure if there was a way to do that and only specify the parameter that I care about.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or rather, the 2nd parameter is an object with a bunch of properties but I'm realy only testing 1 of them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems I had to nest a couple expect.objectContaining() calls, but I cleaned it up (or made it longer and more nested, depending on your point of view 😄 )

expect(requestCallParams.body.access).toEqual(undefined);
});
});

test.concurrent('publish should accept `--access restricted` argument', () => {
return runPublish([], {newVersion: '0.0.1', access: 'restricted'}, 'minimal', config => {
const requestCallParams = config.registries.npm.request.mock.calls[0][1];
expect(requestCallParams.body.access).toEqual('restricted');
});
});

test.concurrent('publish should accept `--access public` argument', () => {
return runPublish([], {newVersion: '0.0.1', access: 'public'}, 'minimal', config => {
const requestCallParams = config.registries.npm.request.mock.calls[0][1];
expect(requestCallParams.body.access).toEqual('public');
});
});

test.concurrent('publish should use publishConfig.access in package manifest', () => {
return runPublish([], {newVersion: '0.0.1'}, 'public', config => {
const requestCallParams = config.registries.npm.request.mock.calls[0][1];
expect(requestCallParams.body.access).toEqual('public');
});
});

test.concurrent('publish should allow `--access` to override publishConfig.access', () => {
return runPublish([], {newVersion: '0.0.1', access: 'restricted'}, 'public', config => {
const requestCallParams = config.registries.npm.request.mock.calls[0][1];
expect(requestCallParams.body.access).toEqual('restricted');
});
});
4 changes: 4 additions & 0 deletions __tests__/fixtures/publish/minimal/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "test",
"version": "0.0.0"
}
7 changes: 7 additions & 0 deletions __tests__/fixtures/publish/public/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "test",
"version": "0.0.0",
"publishConfig": {
"access": "public"
}
}
11 changes: 9 additions & 2 deletions src/cli/commands/publish.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,15 @@ export function hasWrapper(commander: Object, args: Array<string>): boolean {
}

async function publish(config: Config, pkg: any, flags: Object, dir: string): Promise<void> {
let access = flags.access;

// if no access level is provided, check package.json for `publishConfig.access`
// see: https://docs.npmjs.com/files/package.json#publishconfig
if (!access && pkg && pkg.publishConfig && pkg.publishConfig.access) {
access = pkg.publishConfig.access;
}

// validate access argument
const access = flags.access;
if (access && access !== 'public' && access !== 'restricted') {
throw new MessageError(config.reporter.lang('invalidAccess'));
}
Expand Down Expand Up @@ -69,7 +76,7 @@ async function publish(config: Config, pkg: any, flags: Object, dir: string): Pr
// create body
const root = {
_id: pkg.name,
access: flags.access,
access,
name: pkg.name,
description: pkg.description,
'dist-tags': {
Expand Down