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

Pass packageInfos into prepublish, postpublish, and postbump hooks #900

Merged
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
7 changes: 7 additions & 0 deletions change/beachball-b16e6b4f-dc7c-47e6-bd0f-3722b5c2c112.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Beachball passes packageInfos as 4th param of prepublish, postpublish, and postbump hooks",
"packageName": "beachball",
"email": "tronguye@microsoft.com",
"dependentChangeType": "patch"
}
49 changes: 28 additions & 21 deletions src/__e2e__/bump.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -574,20 +574,22 @@ describe('version bumping', () => {

let postBumpCalled = false;

const hooks: BeachballOptions['hooks'] = {
postbump: (packagePath, name, version) => {
postBumpCalled = true;
expect(packagePath.endsWith(path.join('packages', 'pkg-1'))).toBeTruthy();
expect(name).toBe('pkg-1');
expect(version).toBe('1.1.0');

const jsonPath = path.join(packagePath, 'package.json');
expect(fs.readJSONSync(jsonPath).version).toBe('1.1.0');
},
}

await bump({
path: repo.rootPath,
bumpDeps: false,
hooks: {
postbump: (packagePath, name, version) => {
postBumpCalled = true;
expect(packagePath.endsWith(path.join('packages', 'pkg-1'))).toBeTruthy();
expect(name).toBe('pkg-1');
expect(version).toBe('1.1.0');

const jsonPath = path.join(packagePath, 'package.json');
expect(fs.readJSONSync(jsonPath).version).toBe('1.1.0');
},
},
hooks
} as BeachballOptions);

expect(postBumpCalled).toBe(true);
Expand All @@ -607,10 +609,7 @@ describe('version bumping', () => {

let postbumpCalled = false;

await bump({
path: repo.rootPath,
bumpDeps: false,
hooks: {
const hooks: BeachballOptions['hooks'] = {
postbump: async (packagePath, name, version) => {
postbumpCalled = true;
expect(packagePath.endsWith(path.join('packages', 'pkg-1'))).toBeTruthy();
Expand All @@ -620,7 +619,12 @@ describe('version bumping', () => {
const jsonPath = path.join(packagePath, 'package.json');
expect((await fs.readJSON(jsonPath)).version).toBe('1.1.0');
},
},
}

await bump({
path: repo.rootPath,
bumpDeps: false,
hooks,
} as BeachballOptions);

expect(postbumpCalled).toBe(true);
Expand All @@ -638,14 +642,17 @@ describe('version bumping', () => {

repo.push();

const bumpResult = bump({
path: repo.rootPath,
bumpDeps: false,
hooks: {

const hooks: BeachballOptions['hooks'] = {
postbump: async (_packagePath, _name, _version): Promise<void> => {
throw new Error('Foo');
},
},
}

const bumpResult = bump({
path: repo.rootPath,
bumpDeps: false,
hooks
} as BeachballOptions);

await expect(bumpResult).rejects.toThrow('Foo');
Expand Down
4 changes: 3 additions & 1 deletion src/bump/callHook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ export async function callHook(

for (const pkg of affectedPackages) {
const packageInfo = packageInfos[pkg];
await hook(path.dirname(packageInfo.packageJsonPath), packageInfo.name, packageInfo.version);
const packagePath = path.dirname(packageInfo.packageJsonPath);

await hook(packagePath, packageInfo.name, packageInfo.version, packageInfos);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ecraig12345 Should we clone package infos before passing? We're susceptible to side effects by hooks in the current calling pattern

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Marked param as Readonly<> in the typing of hooks. Doesn't technically stop a consumer from casting as writable and causing side effects. lmk what you think

}
}
26 changes: 23 additions & 3 deletions src/types/BeachballOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { AuthType } from './Auth';
import { ChangeInfo, ChangeInfoMultiple, ChangeType } from './ChangeInfo';
import { ChangeFilePromptOptions } from './ChangeFilePrompt';
import { ChangelogOptions } from './ChangelogOptions';
import { PackageInfos } from './PackageInfo';

export type BeachballOptions = CliOptions & RepoOptions & PackageOptions;

Expand Down Expand Up @@ -197,26 +198,45 @@ export interface HooksOptions {
*
* This allows for file modifications which will be reflected in the published package but not be reflected in the
* repository.
*
* @param packagePath The path to the package directory
* @param name The name of the package as defined in package.json
* @param version The post-bump version of the package to be published
* @param packageInfos Metadata about other packages processed by Beachball. Computed post-bump. Readonly.
*/
prepublish?: (packagePath: string, name: string, version: string) => void | Promise<void>;
prepublish?: (packagePath: string, name: string, version: string, packageInfos: Readonly<PackageInfos>) => void | Promise<void>;

/**
* Runs for each package after the publish command.
* Any file changes made in this step will **not** be committed automatically.
*
* @param packagePath The path to the package directory
* @param name The name of the package as defined in package.json
* @param version The post-bump version of the package to be published
* @param packageInfos Metadata about other packages processed by Beachball. Computed post-bump. Readonly.
*/
postpublish?: (packagePath: string, name: string, version: string) => void | Promise<void>;
postpublish?: (packagePath: string, name: string, version: string, packageInfos: Readonly<PackageInfos>) => void | Promise<void>;

/**
* Runs for each package, before writing changelog and package.json updates
* to the filesystem. May be called multiple times during publish.
*
* @param packagePath The path to the package directory
* @param name The name of the package as defined in package.json
* @param version The pre-bump version of the package to be published
*/
prebump?: (packagePath: string, name: string, version: string) => void | Promise<void>;

/**
* Runs for each package, after writing changelog and package.json updates
* to the filesystem. May be called multiple times during publish.
*
* @param packagePath The path to the package directory
* @param name The name of the package as defined in package.json
* @param version The post-bump version of the package to be published
* @param packageInfos Metadata about other packages processed by Beachball. Computed post-bump. Readonly.
*/
postbump?: (packagePath: string, name: string, version: string) => void | Promise<void>;
postbump?: (packagePath: string, name: string, version: string, packageInfos: Readonly<PackageInfos>) => void | Promise<void>;

/**
* Runs once after all bumps to all packages before committing changes
Expand Down