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

V2: Replace Makefile with turborepo #900

Merged
merged 57 commits into from
Jun 26, 2024
Merged

Conversation

timostamm
Copy link
Member

@timostamm timostamm commented Jun 24, 2024

turborepo gives us more control over running tasks in a monorepo, and it caches quite nicely.

For example, when a task is run from a package directory, all its prerequisites run automatically:

cd packages/protoplugin-example
npx turbo run test

Large parts of the setup were taken from connect-playwright-es and adjusted where necessary. CI was split into several jobs and should run in 2 minutes or less.

"./packages/protobuf-example",
"./packages/upstream-protobuf",
"./packages/typescript-compat/*"
"packages/protobuf",
Copy link
Member Author

Choose a reason for hiding this comment

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

Shouldn't be necessary, filed vercel/turbo#8599. It's a harmless change though, and works well enough as a workaround.

Comment on lines +9 to +11
"format": "prettier --write --ignore-unknown '.' '!src/gen/**' '!.turbo'",
"license-header": "license-header",
"lint": "eslint --max-warnings 0 ."
Copy link
Member Author

Choose a reason for hiding this comment

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

Tools like prettier are better run for each package individually with turborepo so that it can cache properly.

},
"dependencies": {
"@bufbuild/buf": "^1.29.0",
"@bufbuild/protobuf": "2.0.0-beta.1",
"@bufbuild/protoplugin": "2.0.0-beta.1",
"@bufbuild/protoc-gen-es": "2.0.0-beta.1",
Copy link
Member Author

Choose a reason for hiding this comment

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

Dependencies replace make prerequisites along with the task dependencies in turbo.json.

md += `|-----------------|----------|------------------------:|-----------------------:|-------------------:|\n`;
for (const row of rows) {
md += `| ${row.name} | ${row.files} | ${formatBytes(row.size)} | ${formatBytes(row.minified)} | ${formatBytes(row.compressed)} |\n`;
export function generateMarkdownTable(rows: Row[]) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Since prettier now runs on markdown, I've updated this code to generate formatted markdown, so that we don't get flip-flopping state.

Comment on lines +1 to +5
{
"$schema": "https://turbo.build/schema.json",
"extends": ["//"],
"tasks": {
"bundle-size": {
Copy link
Member Author

Choose a reason for hiding this comment

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

For tasks specific to a package, it seems most understandable to let it extend turbo.json.

Comment on lines +15 to +22
import {
create,
isFieldSet,
isMessage,
protoInt64,
type MessageInitShape,
type DescMessage,
} from "@bufbuild/protobuf";
Copy link
Member Author

Choose a reason for hiding this comment

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

The new linter config does not allow importing twice from the same source.

import { execSync } from "node:child_process";
import { stdout, stderr, exit } from "node:process";

const typescriptCompatDir = new URL(".", import.meta.url).pathname;
Copy link
Member Author

Choose a reason for hiding this comment

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

This script is replaced with the command turbo run test -F './packages/typescript-compat/*'.

Comment on lines +7 to +15
* Publish protobuf-es
*
* Recommended procedure:
* 1. Set a new version with `npm run setversion 1.2.3`
* 2. Commit and push all changes to a PR, wait for approval.
* 3. Login with `npm login`
* 4. Publish to npmjs.com with `npm run release`
* 5. Merge PR and create a release on GitHub
*/
Copy link
Member Author

@timostamm timostamm Jun 25, 2024

Choose a reason for hiding this comment

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

npm run release replaces make release.

npm run setversion 1.2.3 replaces make setversion SET_VERSION=1.2.3

@timostamm timostamm marked this pull request as ready for review June 25, 2024 13:49
We never have to specify --declarationDir, declarations are already written to the expected location.

We can move --declaration to tsconfig.base.json.
@timostamm timostamm force-pushed the tstamm/replace-make-with-turbo branch from e60d635 to e70447b Compare June 26, 2024 12:12
@timostamm timostamm merged commit d5e1e5c into v2 Jun 26, 2024
18 checks passed
@timostamm timostamm deleted the tstamm/replace-make-with-turbo branch June 26, 2024 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants