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

Respect publishConfig in package manifest #7829

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
115 changes: 101 additions & 14 deletions __tests__/commands/publish.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,34 +51,34 @@ test.concurrent('publish should default access to undefined', () => {
});
});

test.concurrent('publish should accept `--access restricted` argument', () => {
return runPublish([], {newVersion: '0.0.1', access: 'restricted'}, 'minimal', config => {
test.concurrent('publish should use publishConfig.access in package manifest', () => {
return runPublish([], {newVersion: '0.0.1'}, 'public', config => {
expect(config.registries.npm.request).toBeCalledWith(
expect.any(String),
expect.objectContaining({
body: expect.objectContaining({
access: 'restricted',
access: 'public',
}),
}),
);
});
});

test.concurrent('publish should accept `--access public` argument', () => {
return runPublish([], {newVersion: '0.0.1', access: 'public'}, 'minimal', config => {
test.concurrent('publish should accept `--access restricted` argument', () => {
return runPublish([], {newVersion: '0.0.1', access: 'restricted'}, 'minimal', config => {
expect(config.registries.npm.request).toBeCalledWith(
expect.any(String),
expect.objectContaining({
body: expect.objectContaining({
access: 'public',
access: 'restricted',
}),
}),
);
});
});

test.concurrent('publish should use publishConfig.access in package manifest', () => {
return runPublish([], {newVersion: '0.0.1'}, 'public', config => {
test.concurrent('publish should accept `--access public` argument', () => {
return runPublish([], {newVersion: '0.0.1', access: 'public'}, 'minimal', config => {
expect(config.registries.npm.request).toBeCalledWith(
expect.any(String),
expect.objectContaining({
Expand Down Expand Up @@ -148,10 +148,10 @@ test.concurrent('can specify a path without `--new-version`', () => {
});
});

test.concurrent('publish should respect publishConfig.registry ', () => {
test.concurrent('publish should respect publishConfig.registry', () => {
const registry = 'https://registry.myorg.com/';

return runPublish([], {}, 'publish-config-registry', config => {
return runPublish([], {}, 'registry', config => {
expect(config.registries.npm.request).toBeCalledWith(
expect.any(String),
expect.objectContaining({
Expand All @@ -162,11 +162,44 @@ test.concurrent('publish should respect publishConfig.registry ', () => {
});
});

test.concurrent('publish with publishConfig.registry and --registry', () => {
const registry = 'https://registry.myorg.com/';
const registry2 = 'https://registry2.myorg.com/';
test.concurrent(
'publish should allow publishConfig.registry to override "registry"/"@scope:registry" from config',
() => {
const registry = 'https://registry-publish.myorg.com/';

return runPublish([], {}, 'registry-rc', config => {
expect(config.registries.npm.request).toBeCalledWith(
expect.any(String),
expect.objectContaining({
registry,
}),
);
expect(config.registries.npm.getAuthByRegistry).toBeCalledWith(registry);
});
},
);

return runPublish([], {registry: registry2}, 'publish-config-registry', config => {
test.concurrent(
'publish should respect publishConfig["@scope:registry"] (overrides publishConfig.registry if both exist)',
() => {
const registry = 'https://registry-publish.myorg.com/';

return runPublish([], {}, 'scoped-registry', config => {
expect(config.registries.npm.request).toBeCalledWith(
expect.any(String),
expect.objectContaining({
registry,
}),
);
expect(config.registries.npm.getAuthByRegistry).toBeCalledWith(registry);
});
},
);

test.concurrent('publish should allow `--registry` to override publishConfig or config file', () => {
const registry = 'https://registry2.myorg.com/';

return runPublish([], {registry}, 'scoped-registry', config => {
expect(config.registries.npm.request).toBeCalledWith(
expect.any(String),
expect.objectContaining({
Expand All @@ -176,3 +209,57 @@ test.concurrent('publish with publishConfig.registry and --registry', () => {
expect(config.registries.npm.getAuthByRegistry).toBeCalledWith(registry);
});
});

test.concurrent('publish should default tag to "latest"', () => {
const newVersion = '0.0.1';
const tag = 'latest';

return runPublish([], {newVersion}, 'minimal', config => {
expect(config.registries.npm.request).toBeCalledWith(
expect.any(String),
expect.objectContaining({
body: expect.objectContaining({
'dist-tags': {
[tag]: newVersion,
},
}),
}),
);
});
});

test.concurrent('publish should respect publishConfig.tag', () => {
const newVersion = '0.0.1';
const tag = 'next';

return runPublish([], {newVersion}, 'tag', config => {
expect(config.registries.npm.request).toBeCalledWith(
expect.any(String),
expect.objectContaining({
body: expect.objectContaining({
'dist-tags': {
[tag]: newVersion,
},
}),
}),
);
});
});

test.concurrent('publish should allow `--tag` to override publishConfig.tag', () => {
const newVersion = '0.0.1';
const tag = 'beta';

return runPublish([], {newVersion, tag}, 'tag', config => {
expect(config.registries.npm.request).toBeCalledWith(
expect.any(String),
expect.objectContaining({
body: expect.objectContaining({
'dist-tags': {
[tag]: newVersion,
},
}),
}),
);
});
});
1 change: 1 addition & 0 deletions __tests__/fixtures/publish/registry-rc/.yarnrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"@scope:registry" "https://registry.myorg.com/"
7 changes: 7 additions & 0 deletions __tests__/fixtures/publish/registry-rc/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "@scope/test",
"version": "0.0.0",
"publishConfig": {
"registry": "https://registry-publish.myorg.com/"
}
}
1 change: 1 addition & 0 deletions __tests__/fixtures/publish/scoped-registry/.yarnrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"@scope:registry" "https://registry.myorg.com/"
8 changes: 8 additions & 0 deletions __tests__/fixtures/publish/scoped-registry/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "@scope/test",
"version": "0.0.0",
"publishConfig": {
"@scope:registry": "https://registry-publish.myorg.com/",
"registry": "https://registry.npmjs.org/"
}
}
7 changes: 7 additions & 0 deletions __tests__/fixtures/publish/tag/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "test",
"version": "0.0.0",
"publishConfig": {
"tag": "next"
}
}
66 changes: 50 additions & 16 deletions src/cli/commands/publish.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,55 @@ export function hasWrapper(commander: Object, args: Array<string>): boolean {
return true;
}

async function publish(config: Config, pkg: any, flags: Object, dir: string): Promise<void> {
let access = flags.access;
function getPublishConfigAccess(pkg: any, flags: Object): ?string {
if (typeof flags.access === 'string') {
return flags.access;
}
if (pkg && pkg.publishConfig && typeof pkg.publishConfig.access === 'string') {
return pkg.publishConfig.access;
}
return undefined;
}

// 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;
function getPublishConfigRegistry(config: Config, pkg: any, flags: Object): string {
// CLI flag has the highest priority.
if (typeof flags.registry === 'string') {
return flags.registry;
}
// if `publishConfig` exists, it overrides the default registry settings.
if (pkg && pkg.publishConfig) {
const publishConfig = pkg.publishConfig;
const scope = config.registries.npm.getScope(pkg.name);
// if the package is scoped, scoped registry in `publishConfig` has priority 2.
if (scope) {
const scopedRegistry = publishConfig[`${scope}:registry`];
if (typeof scopedRegistry === 'string') {
return scopedRegistry;
}
}
// use `publishConfig.registry` even if the package is scoped, as long as it doesn't have scoped registry
const unscopedRegistry = publishConfig.registry;
if (typeof unscopedRegistry === 'string') {
return unscopedRegistry;
}
}
return '';
}

function getPublishConfigTag(pkg: any, flags: Object): string {
if (typeof flags.tag === 'string') {
return flags.tag;
}
if (pkg && pkg.publishConfig && typeof pkg.publishConfig.tag === 'string') {
return pkg.publishConfig.tag;
}
return 'latest';
}

async function publish(config: Config, pkg: any, flags: Object, dir: string): Promise<void> {
// check package.json for `publishConfig.access`, override with `--access` flag
// see: https://docs.npmjs.com/files/package.json#publishconfig
const access = getPublishConfigAccess(pkg, flags);

// validate access argument
if (access && access !== 'public' && access !== 'restricted') {
Expand Down Expand Up @@ -74,7 +115,7 @@ async function publish(config: Config, pkg: any, flags: Object, dir: string): Pr
}
}

const tag = flags.tag || 'latest';
const tag = getPublishConfigTag(pkg, flags);
const tbName = `${pkg.name}-${pkg.version}.tgz`;
const tbURI = `${pkg.name}/-/${tbName}`;

Expand Down Expand Up @@ -111,7 +152,7 @@ async function publish(config: Config, pkg: any, flags: Object, dir: string): Pr
// publish package
try {
await config.registries.npm.request(NpmRegistry.escapeName(pkg.name), {
registry: pkg && pkg.publishConfig && pkg.publishConfig.registry,
registry: getPublishConfigRegistry(config, pkg, flags),
method: 'PUT',
body: root,
});
Expand Down Expand Up @@ -141,7 +182,6 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg
}

// validate package fields that are required for publishing
// $FlowFixMe
const pkg = await config.readRootManifest();
if (pkg.private) {
throw new MessageError(reporter.lang('publishPrivate'));
Expand All @@ -150,18 +190,12 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg
throw new MessageError(reporter.lang('noName'));
}

let registry: string = '';

if (pkg && pkg.publishConfig && pkg.publishConfig.registry) {
registry = pkg.publishConfig.registry;
}

reporter.step(1, 4, reporter.lang('bumpingVersion'));
const commitVersion = await setVersion(config, reporter, flags, [], false);

//
reporter.step(2, 4, reporter.lang('loggingIn'));
const revoke = await getToken(config, reporter, pkg.name, flags, registry);
const revoke = await getToken(config, reporter, pkg.name, flags, getPublishConfigRegistry(config, pkg, flags));

//
reporter.step(3, 4, reporter.lang('publishing'));
Expand Down
6 changes: 6 additions & 0 deletions src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,12 @@ export type Manifest = {
fresh?: boolean,

prebuiltVariants?: {[filename: string]: string},

publishConfig?: {
access?: string,
registry?: string,
tag?: string,
},
};

//
Expand Down