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

pre-commit: Keep prettier version in sync; add eslint fixer #1400

Closed
wants to merge 6 commits into from
Closed
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
28 changes: 26 additions & 2 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,34 @@ repos:
# document contents. For example
# packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/languages/ruby/changeCondition.yml
exclude: ^packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/.*/[^/]*\.yml$
- repo: https://github.com/pre-commit/mirrors-prettier
rev: "v2.7.1"
- repo: local
hooks:
- id: eslint
auscompgeek marked this conversation as resolved.
Show resolved Hide resolved
name: eslint
files: \.(ts|tsx)$
language: node
entry: eslint --fix
additional_dependencies:
- "@typescript-eslint/[email protected]"
- "@typescript-eslint/[email protected]"
- [email protected]
- [email protected]
- [email protected]
- [email protected]
- [email protected]
- [email protected]
- [email protected]
args: []
- repo: local
hooks:
- id: prettier
name: prettier
types: [text]
language: node
entry: prettier --write --list-different --ignore-unknown
additional_dependencies:
- [email protected]
args: []
- repo: https://github.com/ikamensh/flynt/
rev: "0.78"
hooks:
Expand Down
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"watch": "tsc --build --watch",
"init-vscode-sandbox": "pnpm --filter=@cursorless/cursorless-vscode-core init-launch-sandbox",
"meta-updater:base": "pnpm --filter=@cursorless/meta-updater build && meta-updater",
"meta-updater": "pnpm run meta-updater:base && pnpm -r exec prettier --write tsconfig.json package.json",
"meta-updater": "pnpm run meta-updater:base && pnpm -r exec prettier --write tsconfig.json package.json && pnpm -w exec prettier --write .pre-commit-config.yaml",
"lint:meta": "pnpm run meta-updater:base --test",
"lint": "pnpm run lint:meta && syncpack list-mismatches && pnpm run lint:ts",
"lint:ts": "eslint packages --ext ts",
Expand Down Expand Up @@ -40,7 +40,8 @@
},
"pnpm": {
"patchedDependencies": {
"@docusaurus/[email protected]": "patches/@[email protected]"
"@docusaurus/[email protected]": "patches/@[email protected]",
"@pnpm/[email protected]": "patches/@[email protected]"
Copy link
Member Author

Choose a reason for hiding this comment

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

Patching until pnpm/meta-updater#10 is fixed

},
"peerDependencyRules": {
"ignoreMissing": [
Expand Down
7 changes: 6 additions & 1 deletion packages/meta-updater/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,12 @@
"@pnpm/logger": "^5.0.0",
"@pnpm/types": "8.9.0",
"@types/normalize-path": "^3.0.0",
"lodash": "^4.17.21",
"normalize-path": "^3.0.0",
"path-exists": "^4.0.0",
"type-fest": "3.6.1"
"ramda": "0.29.0",
"type-fest": "3.6.1",
"yaml": "2.2.1"
},
"main": "./out/index.js",
"types": "./out/index.d.ts",
Expand All @@ -27,6 +30,8 @@
}
},
"devDependencies": {
"@types/lodash": "4.14.181",
"@types/ramda": "0.28.23",
"esbuild": "^0.17.11"
}
}
29 changes: 29 additions & 0 deletions packages/meta-updater/src/formats.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { createFormat } from "@pnpm/meta-updater";
import { readFile, writeFile } from "fs/promises";
import { equals } from "ramda";
import yaml from "yaml";

export const formats = {
/**
* A format that today we just use for .pre-commit-config.yaml files. This is a yaml file, but we
* need to preserve comments, so we use the `yaml` library's document representation instead of
* parsing it into a plain js object.
*/
[".yaml"]: createFormat({
async read({ resolvedPath }) {
return yaml.parseDocument(await readFile(resolvedPath, "utf-8")).clone();
},
update(actual, updater, options) {
return updater(actual, options);
},
equal(expected, actual) {
return equals(actual.toJS(), expected.toJS());
},
async write(expected, { resolvedPath }) {
await writeFile(resolvedPath, expected.toString());
},
clone(content) {
return content == null ? content : content.clone();
},
}),
};
36 changes: 36 additions & 0 deletions packages/meta-updater/src/getPackageDeps.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import normalizePath from "normalize-path";
import path from "path";
import { Lockfile } from "@pnpm/lockfile-file";

/**
* Get the dependencies of the given package from the pnpm lockfile.
* @param workspaceDir The root of the workspace
* @param packageDir The directory of the package whose dependencies we are
* retrieving
* @param pnpmLockfile The parsed pnpm lockfile
* @returns A map of package names to package specs for the dependencies of the
* given package
*/
export function getPackageDeps(
workspaceDir: string,
packageDir: string,
pnpmLockfile: Lockfile,
) {
const pathFromRootToPackage =
packageDir === workspaceDir
? "."
: normalizePath(path.relative(workspaceDir, packageDir));

/** Info about package dependencies gleaned from lock file. */
const lockFilePackageInfo = pnpmLockfile.importers[pathFromRootToPackage];
if (!lockFilePackageInfo) {
// Raise an error here because there should always be an entry in the lockfile.
throw new Error(`No importer found for ${pathFromRootToPackage}`);
}

const deps = {
...lockFilePackageInfo.dependencies,
...lockFilePackageInfo.devDependencies,
};
return deps;
}
21 changes: 21 additions & 0 deletions packages/meta-updater/src/mergeStrict.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* Merge the input objects, throwing an error if there are any conflicts.
*/
export function mergeStrict(
...objs: Record<string, string>[]
): Record<string, string> {
const result: Record<string, string> = {};

for (const obj of objs) {
for (const [key, value] of Object.entries(obj)) {
if (result[key] !== undefined && result[key] !== value) {
throw new Error(
`Conflicting versions for ${key}: ${result[key]} and ${value}`,
);
}
result[key] = value;
}
}

return result;
}
10 changes: 8 additions & 2 deletions packages/meta-updater/src/metaUpdater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import { createUpdateOptions } from "@pnpm/meta-updater";
import { Context } from "./Context";
import { updatePackageJson } from "./updatePackageJson";
import { updateTSConfig } from "./updateTSConfig";
import { updatePreCommit } from "./updatePreCommit";
import { formats } from "./formats";

export const updater = async (workspaceDir: string) => {
const pnpmLockfile = await readWantedLockfile(workspaceDir, {
Expand All @@ -23,7 +25,11 @@ export const updater = async (workspaceDir: string) => {
};

return createUpdateOptions({
["package.json"]: updatePackageJson.bind(null, context),
["tsconfig.json"]: updateTSConfig.bind(null, context),
files: {
["package.json"]: updatePackageJson.bind(null, context),
["tsconfig.json"]: updateTSConfig.bind(null, context),
[".pre-commit-config.yaml"]: updatePreCommit.bind(null, context),
},
formats,
});
};
126 changes: 126 additions & 0 deletions packages/meta-updater/src/updatePreCommit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
import type { FormatPluginFnOptions } from "@pnpm/meta-updater";
import { Document, ParsedNode } from "yaml";
import { Context } from "./Context";
import { Lockfile } from "@pnpm/lockfile-file";
import { pickBy } from "lodash";
import { mergeStrict } from "./mergeStrict";

/**
* Subset of the .pre-commit-config.yaml schema that we care about.
*/
interface PreCommitConfig {
repos: {
hooks: {
id: string;
// eslint-disable-next-line @typescript-eslint/naming-convention
additional_dependencies: string[];
}[];
}[];
}

/**
* Given a .pre-commit-config.yaml, update it to ensure that the versions of our
* hooks match the corresponding package versions in package.json. This
* function is called by the pnpm `meta-updater` plugin either to check if the
* .pre-commit-config.yaml is up to date or to update it, depending on flags.
* @param context Contains context such as workspace dir and parsed pnpm
* lockfile
* @param rawInput The input .pre-commit-config.yaml that should be checked /
* updated. This is a parsed yaml document in the `yaml` library's document
* representation; not a plain js object like you'd get from a json parser. We
* need it like this so that we can preserve comments.
* @param options Extra information provided by pnpm; mostly just the directory
* of the package whose .pre-commit-config.yaml we are updating
* @returns The updated .pre-commit-config.yaml
*/
export async function updatePreCommit(
{ workspaceDir, pnpmLockfile }: Context,
rawInput: Document<ParsedNode> | null,
options: FormatPluginFnOptions,
): Promise<Document<ParsedNode> | null> {
if (rawInput == null) {
return null;
}
/** Directory of the package whose .pre-commit-config.yaml we are updating */
const packageDir = options.dir;

if (packageDir !== workspaceDir) {
throw new Error("updatePreCommit should only be called on root");
}

updateHook(pnpmLockfile, rawInput, "prettier", (name) => name === "prettier");
updateHook(
pnpmLockfile,
rawInput,
"eslint",
(name) => name.includes("eslint") || name === "typescript",
);

return rawInput;
}

/**
* Updates the additional_dependencies of a hook in a .pre-commit-config.yaml to
* match the versions from the lockfile.
* @param pnpmLockfile The pnpm lockfile, which contains the versions of all
* packages in the workspace
* @param rawInput The input .pre-commit-config.yaml that should be checked /
* updated
* @param hookId The id of the hook to update
* @param packageMatcher A function that returns true if the given package name
* should be added to the hook's additional_dependencies
*/
function updateHook(
pnpmLockfile: Lockfile,
rawInput: Document<ParsedNode>,
hookId: string,
packageMatcher: (name: string) => boolean,
) {
// Find all packages that match the given packageMatcher in the dependencies
// of any package in the workspace
const packages = Object.entries(
mergeStrict(
...Object.values(pnpmLockfile.importers)
.flatMap((packageInfo) => [
packageInfo.dependencies ?? {},
packageInfo.devDependencies ?? {},
])
.map((deps) => pickBy(deps, (_, key) => packageMatcher(key))),
),
);

// Find the hook in the .pre-commit-config.yaml. Easier to grab the indices
// from the raw js representation so that we can just use `setIn` to update
// the hook
const desiredHooks = (rawInput.toJS() as PreCommitConfig).repos
.flatMap(({ hooks }, repoIndex) =>
hooks.map((hook, hookIndex) => ({ hook, repoIndex, hookIndex })),
)
.filter(({ hook }) => hook.id === hookId);

if (desiredHooks.length === 0) {
throw new Error(`No ${hookId} hook found`);
}

if (desiredHooks.length > 1) {
throw new Error(`Multiple ${hookId} hooks found`);
}

const { repoIndex, hookIndex } = desiredHooks[0];

rawInput.setIn(
["repos", repoIndex, "hooks", hookIndex, "additional_dependencies"],
rawInput.createNode(
packages
.map(([name, version]) => {
if (version.includes("(")) {
// pnpm includes the integrity hash in the version, which we don't
// need here
version = version.slice(0, version.indexOf("("));
}
return `${name}@${version}`;
})
.sort(),
),
);
}
16 changes: 2 additions & 14 deletions packages/meta-updater/src/updateTSConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import exists from "path-exists";
import { TsConfigJson } from "type-fest";
import { toPosixPath } from "./toPosixPath";
import { Context } from "./Context";
import { getPackageDeps } from "./getPackageDeps";

/**
* Given a tsconfig.json, update it to match our conventions. This function is
Expand Down Expand Up @@ -41,24 +42,11 @@ export async function updateTSConfig(
};
}

const pathFromRootToPackage = normalizePath(
path.relative(workspaceDir, packageDir),
);
const pathFromPackageToRoot = normalizePath(
path.relative(packageDir, workspaceDir),
);

/** Info about package dependencies gleaned from lock file. */
const lockFilePackageInfo = pnpmLockfile.importers[pathFromRootToPackage];
if (!lockFilePackageInfo) {
// Raise an error here because there should always be an entry in the lockfile.
throw new Error(`No importer found for ${pathFromRootToPackage}`);
}

const deps = {
...lockFilePackageInfo.dependencies,
...lockFilePackageInfo.devDependencies,
};
const deps = getPackageDeps(workspaceDir, packageDir, pnpmLockfile);

/** Computed tsconfig.json references based on dependencies. */
const references = [] as Array<{ path: string }>;
Expand Down
27 changes: 27 additions & 0 deletions patches/@[email protected]
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
diff --git a/lib/index.js b/lib/index.js
index d24112f5fedba5a9d7c5bd85221652a8f0bf09e1..2af95628f80926b8c4c9c774d9135a95b465959c 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -48,7 +48,8 @@ export async function performUpdates(workspaceDir, updateParam, opts) {
_writeProjectManifest: writeProjectManifest,
};
const actual = (await fileExists(resolvedPath)) ? await formatPlugin.read(formatHandlerOptions) : null;
- const expected = await formatPlugin.update(clone(actual), updateFile, formatHandlerOptions);
+ const customClone = formatPlugin.clone == null ? clone : formatPlugin.clone;
+ const expected = await formatPlugin.update(customClone(actual), updateFile, formatHandlerOptions);
const equal = (actual == null && expected == null) ||
(actual != null && expected != null && (await formatPlugin.equal(expected, actual, formatHandlerOptions)));
if (equal) {
diff --git a/lib/updater/formatPlugin.d.ts b/lib/updater/formatPlugin.d.ts
index 887845721b09e17b95dd0789a5db8de72c04a654..2e47ba30b25661cec37d3ab1dcca1d8908a1eb21 100644
--- a/lib/updater/formatPlugin.d.ts
+++ b/lib/updater/formatPlugin.d.ts
@@ -12,6 +12,8 @@ export interface FormatPlugin<Content> {
equal(expected: Content, actual: Content, options: FormatPluginFnOptions): PromiseOrValue<boolean>;
/** Called only if write is required (`--test` isn't specified, `expected != null` and `expected` is not equal to `actual`) */
write(expected: Content, options: FormatPluginFnOptions): PromiseOrValue<void>;
+ /** Can be used to override the built-in clone functionality */
+ clone?(content: Content): PromiseOrValue<Content>;
}
export interface FormatPluginFnOptions {
file: string;
Loading