diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 0000000..ee4ce50 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,7 @@ +/src/ @SandroMaglione +/scripts/ @SandroMaglione +/.github/ @SandroMaglione +/package.json @SandroMaglione +/pnpm-lock.yaml @SandroMaglione +/tsconfig.build.json @SandroMaglione +/.changeset/config.json @SandroMaglione diff --git a/.github/ISSUE_TEMPLATE/proposal.md b/.github/ISSUE_TEMPLATE/proposal.md new file mode 100644 index 0000000..61c583d --- /dev/null +++ b/.github/ISSUE_TEMPLATE/proposal.md @@ -0,0 +1,25 @@ +--- +name: Change proposal +about: Discuss a problem or API change before implementation +title: "" +labels: "" +assignees: "" +--- + +## Problem or use case + +What are you trying to do, and what is difficult or impossible with the current API? + +## Proposed direction + +Describe the behavior or public API you would like to add or change. + +## Example + +Show a small example of how the proposed API would be used. + +## Additional context + +Include alternatives, constraints, or related issues. + +Please wait for the proposal to be discussed before opening a pull request. diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 0000000..a837a42 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,19 @@ +## Summary + +- + +## Changeset + +- [ ] Added or updated for a library or package-metadata change +- [ ] Not required because this PR does not change `src/` or `package.json` + +## Validation + +- [ ] `pnpm check` +- [ ] Relevant example checks, when examples changed +- [ ] Reviewed the automated type-performance report, when the public TypeScript API or inference changed + + diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4e612c3..b544f87 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,6 +5,10 @@ on: push: branches: [main] +concurrency: + group: ci-${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + permissions: contents: read @@ -21,46 +25,65 @@ jobs: - run: pnpm install --frozen-lockfile - run: pnpm check - pokemon-example: + changeset-policy: + if: github.event_name == 'pull_request' runs-on: ubuntu-latest - defaults: - run: - working-directory: examples/pokemon steps: - uses: actions/checkout@v7 - - uses: pnpm/action-setup@v6 - with: - package_json_file: examples/pokemon/package.json - - uses: actions/setup-node@v7 with: - node-version: 24 - cache: pnpm - cache-dependency-path: | - pnpm-lock.yaml - examples/pokemon/pnpm-lock.yaml - - run: pnpm --dir ../.. install --frozen-lockfile - - run: pnpm --dir ../.. build - - run: pnpm install --frozen-lockfile - - run: pnpm check + fetch-depth: 0 + - run: >- + node scripts/check-changeset.mjs + --base ${{ github.event.pull_request.base.sha }} + --head ${{ github.event.pull_request.head.sha }} + + discover-examples: + runs-on: ubuntu-latest + outputs: + matrix: ${{ steps.examples.outputs.matrix }} + steps: + - uses: actions/checkout@v7 + - id: examples + run: echo "matrix=$(node scripts/list-examples.mjs)" >> "$GITHUB_OUTPUT" - platformer-example: + example: + needs: discover-examples + strategy: + fail-fast: false + matrix: + include: ${{ fromJSON(needs.discover-examples.outputs.matrix) }} + name: example (${{ matrix.example }}) runs-on: ubuntu-latest defaults: run: - working-directory: examples/platformer + working-directory: ${{ matrix.directory }} steps: - uses: actions/checkout@v7 - uses: pnpm/action-setup@v6 with: - package_json_file: examples/platformer/package.json + package_json_file: ${{ matrix.directory }}/package.json - uses: actions/setup-node@v7 with: node-version: 24 cache: pnpm cache-dependency-path: | pnpm-lock.yaml - examples/platformer/pnpm-lock.yaml + ${{ matrix.directory }}/pnpm-lock.yaml - run: pnpm --dir ../.. install --frozen-lockfile - run: pnpm --dir ../.. build - run: pnpm install --frozen-lockfile - run: pnpm check + + examples: + if: always() + needs: [discover-examples, example] + name: examples + runs-on: ubuntu-latest + steps: + - name: Require every example check to pass + env: + DISCOVERY_RESULT: ${{ needs.discover-examples.result }} + EXAMPLE_RESULT: ${{ needs.example.result }} + run: | + test "$DISCOVERY_RESULT" = "success" + test "$EXAMPLE_RESULT" = "success" diff --git a/.github/workflows/type-performance-comment.yml b/.github/workflows/type-performance-comment.yml new file mode 100644 index 0000000..2291c60 --- /dev/null +++ b/.github/workflows/type-performance-comment.yml @@ -0,0 +1,102 @@ +name: Type performance comment + +on: + workflow_run: + workflows: [Type performance] + types: [completed] + +concurrency: + group: type-performance-comment-${{ github.event.workflow_run.head_repository.id }}-${{ github.event.workflow_run.head_branch }} + cancel-in-progress: true + +permissions: + actions: read + contents: read + issues: write + pull-requests: read + +jobs: + comment: + if: >- + github.event.workflow_run.event == 'pull_request' && + github.event.workflow_run.conclusion == 'success' + runs-on: ubuntu-latest + steps: + - name: Check out trusted reporting code + uses: actions/checkout@v7 + + - name: Download performance report + uses: actions/download-artifact@v8 + with: + name: type-performance-report + path: report + github-token: ${{ secrets.GITHUB_TOKEN }} + run-id: ${{ github.event.workflow_run.id }} + + - name: Render report from validated benchmark data + run: >- + node scripts/compare-type-performance.mjs + report/before.json + report/after.json + > "$RUNNER_TEMP/report.md" + + - name: Create or update pull request comment + uses: actions/github-script@v9 + env: + REPORT_PATH: ${{ runner.temp }}/report.md + with: + script: | + const fs = require("node:fs") + const marker = "" + const report = fs.readFileSync(process.env.REPORT_PATH, "utf8") + + if (Buffer.byteLength(report, "utf8") > 60_000) { + core.setFailed("Type-performance report exceeds the safe comment size") + return + } + + const run = context.payload.workflow_run + let pull = run.pull_requests?.[0] + + if (pull === undefined) { + try { + const associated = await github.rest.repos.listPullRequestsAssociatedWithCommit({ + ...context.repo, + commit_sha: run.head_sha + }) + pull = associated.data.find((candidate) => + candidate.base.repo.full_name === `${context.repo.owner}/${context.repo.repo}` + ) + } catch (error) { + core.warning(`Unable to look up a pull request for ${run.head_sha}: ${error.message}`) + } + } + + if (pull === undefined) { + core.notice("No pull request is associated with this workflow run") + return + } + + const body = `${marker}\n${report}` + const comments = await github.paginate(github.rest.issues.listComments, { + ...context.repo, + issue_number: pull.number, + per_page: 100 + }) + const previous = comments.find((comment) => + comment.user?.type === "Bot" && comment.body?.startsWith(marker) + ) + + if (previous === undefined) { + await github.rest.issues.createComment({ + ...context.repo, + issue_number: pull.number, + body + }) + } else { + await github.rest.issues.updateComment({ + ...context.repo, + comment_id: previous.id, + body + }) + } diff --git a/.github/workflows/type-performance.yml b/.github/workflows/type-performance.yml new file mode 100644 index 0000000..b6be4d7 --- /dev/null +++ b/.github/workflows/type-performance.yml @@ -0,0 +1,68 @@ +name: Type performance + +on: + pull_request: + +concurrency: + group: type-performance-${{ github.event.pull_request.number }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + type-performance: + name: type-performance + runs-on: ubuntu-latest + steps: + - name: Check out base + uses: actions/checkout@v7 + with: + path: base + ref: ${{ github.event.pull_request.base.sha }} + + - name: Check out pull request + uses: actions/checkout@v7 + with: + path: head + repository: ${{ github.event.pull_request.head.repo.full_name }} + ref: ${{ github.event.pull_request.head.sha }} + + - uses: pnpm/action-setup@v6 + with: + package_json_file: head/package.json + + - uses: actions/setup-node@v7 + with: + node-version: 24 + cache: pnpm + cache-dependency-path: | + base/pnpm-lock.yaml + head/pnpm-lock.yaml + + - name: Install and build base + run: | + pnpm --dir base install --frozen-lockfile + pnpm --dir base build + + - name: Install and build pull request + run: | + pnpm --dir head install --frozen-lockfile + pnpm --dir head build + + - name: Measure base and pull request + run: | + node head/scripts/type-performance.mjs --root base --json --allow-missing > "$RUNNER_TEMP/before.json" + node head/scripts/type-performance.mjs --root head --json > "$RUNNER_TEMP/after.json" + node head/scripts/compare-type-performance.mjs "$RUNNER_TEMP/before.json" "$RUNNER_TEMP/after.json" > "$RUNNER_TEMP/report.md" + cat "$RUNNER_TEMP/report.md" >> "$GITHUB_STEP_SUMMARY" + + - name: Upload report for the comment workflow + uses: actions/upload-artifact@v7 + with: + name: type-performance-report + path: | + ${{ runner.temp }}/before.json + ${{ runner.temp }}/after.json + if-no-files-found: error + retention-days: 7 diff --git a/AGENTS.md b/AGENTS.md index 73de8dc..9d613f2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,21 +6,29 @@ The goal is eventually to merge this inside the core of the `effect` library, so Make architectural decisions for the long term. Do not accept a stopgap that only works for now and is meant to be replaced later. -## Feature verification workflow +## Verification -Before implementing a feature, run: +Run: ```sh -pnpm perf:types +pnpm check ``` -Record the type-performance results as the baseline for the feature. - -After implementing the feature, run: +For changes that can affect the public TypeScript API or its inference, also run: ```sh -pnpm typecheck pnpm perf:types ``` -Compare the final type-performance results with the baseline. When reporting the completed work, include the before and after results and call out the additional type-instantiation cost of the feature, including regressions or improvements. +When an example changes, run its own check from the example directory: + +```sh +pnpm check +``` + +Every package directly below `examples/` must have a `check` script and a committed lockfile. + +## Pull request conventions + +- Add or update a changeset for changes under `src/` or changes to `package.json`. +- Fill in the pull request template, including the validation performed and the changeset decision. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..0313df2 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,5 @@ +# Contributing + +This project generally does not accept unsolicited pull requests. Open an issue first describing the problem or use case and, for API changes, the public API you want to add or change. + +Wait for the proposal to be discussed and accepted before starting an implementation or opening a pull request. Pull requests without prior agreement may be closed. diff --git a/README.md b/README.md index 85987c3..356246a 100644 --- a/README.md +++ b/README.md @@ -495,6 +495,9 @@ Individual commands are available for `build`, `test`, `test:types`, packs the package, imports all public entrypoints, and compiles a strict TypeScript consumer with `skipLibCheck: false`. +Read [CONTRIBUTING.md](./CONTRIBUTING.md) before proposing a change. Pull +requests receive an automated base-versus-head type-instantiation report. + ## Examples The [platformer statechart example](./examples/platformer) is a playable SVG @@ -510,6 +513,11 @@ invoked child statecharts, typed emissions, and Atom reactivity. It uses a local `file:` dependency on this package while retaining an isolated dependency graph, lockfile, build, and CI job. +The [playground](./examples/playground) collects focused interactive examples +for traffic lights, turnstiles, media players, microwaves, and worker-backed +machines. CI discovers every direct package under `examples/` and runs its +`check` script automatically. + ## Releases Add a changeset with `pnpm changeset`. CI validates frozen installation and the diff --git a/scripts/check-changeset.mjs b/scripts/check-changeset.mjs new file mode 100644 index 0000000..4076fcc --- /dev/null +++ b/scripts/check-changeset.mjs @@ -0,0 +1,58 @@ +import { spawnSync } from "node:child_process" + +const options = { + base: "origin/main", + head: "HEAD" +} + +for (let index = 2; index < process.argv.length; index += 1) { + const argument = process.argv[index] + if (argument === "--base" || argument === "--head") { + const value = process.argv[index + 1] + if (value === undefined) { + throw new Error(`${argument} requires a git revision`) + } + options[argument.slice(2)] = value + index += 1 + } else { + throw new Error(`Unknown argument: ${argument}`) + } +} + +const diff = (filters) => { + const result = spawnSync( + "git", + ["diff", "--name-only", "-z", `--diff-filter=${filters}`, `${options.base}...${options.head}`], + { encoding: "utf8" } + ) + if (result.status !== 0) { + throw new Error(result.stderr.trim() || "Unable to inspect the pull request diff") + } + return result.stdout.split("\0").filter(Boolean) +} + +const changedFiles = diff("ACDMRTUXB") +const releaseFiles = changedFiles.filter( + (path) => path.startsWith("src/") || path === "package.json" +) + +if (releaseFiles.length === 0) { + console.log("No library or package-metadata changes require a changeset.") + process.exit(0) +} + +const changedChangesets = diff("AM").filter( + (path) => /^\.changeset\/(?!README\.md$)[^/]+\.md$/.test(path) +) + +if (changedChangesets.length === 0) { + console.error("Library changes require a new or updated changeset.") + console.error("Files requiring a changeset:") + for (const path of releaseFiles) { + console.error(`- ${path}`) + } + console.error("Run `pnpm changeset`, commit the generated file, and push again.") + process.exit(1) +} + +console.log(`Changeset policy satisfied by: ${changedChangesets.join(", ")}`) diff --git a/scripts/compare-type-performance.mjs b/scripts/compare-type-performance.mjs new file mode 100644 index 0000000..bd916d4 --- /dev/null +++ b/scripts/compare-type-performance.mjs @@ -0,0 +1,159 @@ +import { readFileSync } from "node:fs" + +const [beforePath, afterPath] = process.argv.slice(2) + +if (beforePath === undefined || afterPath === undefined) { + throw new Error("Usage: node scripts/compare-type-performance.mjs ") +} + +const readReport = (path) => { + const source = readFileSync(path, "utf8") + if (Buffer.byteLength(source, "utf8") > 100_000) { + throw new Error(`Type-performance report is too large: ${path}`) + } + + const report = JSON.parse(source) + if (report.schemaVersion !== 1 || !Array.isArray(report.scenarios)) { + throw new Error(`Unsupported type-performance report: ${path}`) + } + + if ( + typeof report.typescriptVersion !== "string" || + !/^[0-9A-Za-z.+-]{1,40}$/.test(report.typescriptVersion) + ) { + throw new Error(`Invalid TypeScript version in report: ${path}`) + } + if (report.scenarios.length > 100) { + throw new Error(`Too many type-performance scenarios in report: ${path}`) + } + for (const scenario of report.scenarios) { + if ( + typeof scenario.id !== "string" || + scenario.id.length > 100 || + typeof scenario.label !== "string" || + scenario.label.length > 200 || + !Number.isFinite(scenario.instantiations) || + scenario.instantiations < 0 || + ( + scenario.marginalInstantiations !== null && + !Number.isFinite(scenario.marginalInstantiations) + ) || + !Number.isFinite(scenario.checkTimeSeconds) || + scenario.checkTimeSeconds < 0 + ) { + throw new Error(`Invalid type-performance scenario in report: ${path}`) + } + } + return report +} + +const before = readReport(beforePath) +const after = readReport(afterPath) +const beforeScenarios = new Map(before.scenarios.map((scenario) => [scenario.id, scenario])) +const afterScenarios = new Map(after.scenarios.map((scenario) => [scenario.id, scenario])) +const scenarioIds = [ + ...after.scenarios.map((scenario) => scenario.id), + ...before.scenarios + .map((scenario) => scenario.id) + .filter((id) => !afterScenarios.has(id)) +] + +const integer = new Intl.NumberFormat("en-US", { maximumFractionDigits: 0 }) +const seconds = new Intl.NumberFormat("en-US", { + minimumFractionDigits: 2, + maximumFractionDigits: 2 +}) + +const formatInteger = (value) => value === undefined || value === null ? "—" : integer.format(value) + +const formatDifference = (beforeValue, afterValue) => { + if (beforeValue === undefined || beforeValue === null) { + return afterValue === undefined || afterValue === null ? "—" : "new" + } + if (afterValue === undefined || afterValue === null) { + return "removed" + } + + const difference = afterValue - beforeValue + const sign = difference > 0 ? "+" : "" + const percentage = beforeValue === 0 ? undefined : difference / beforeValue * 100 + const percentageText = percentage === undefined + ? "" + : ` (${sign}${percentage.toFixed(1)}%)` + return `${sign}${integer.format(difference)}${percentageText}` +} + +const formatTime = (value) => value === undefined ? "—" : `${seconds.format(value)}s` +const markdownLabel = (scenario) => { + if (scenario === undefined) { + return "unknown" + } + return `\`${scenario.label.replaceAll("`", "\\`").replaceAll("|", "\\|")}\`` +} + +const lines = [ + "## Type performance", + "", + before.typescriptVersion === after.typescriptVersion + ? `Measured with TypeScript ${after.typescriptVersion} and \`skipLibCheck=true\`.` + : `Base uses TypeScript ${before.typescriptVersion}; PR uses TypeScript ${after.typescriptVersion}. Results include the compiler-version change.`, + "", + "| Scenario | Base | PR | Difference |", + "| --- | ---: | ---: | ---: |" +] + +for (const id of scenarioIds) { + const beforeScenario = beforeScenarios.get(id) + const afterScenario = afterScenarios.get(id) + lines.push( + `| ${markdownLabel(afterScenario ?? beforeScenario)} | ${formatInteger(beforeScenario?.instantiations)} | ${formatInteger(afterScenario?.instantiations)} | ${formatDifference(beforeScenario?.instantiations, afterScenario?.instantiations)} |` + ) +} + +lines.push( + "", + "Marginal instantiations are measured against the matching setup without that API call:", + "", + "| Scenario | Base | PR | Difference |", + "| --- | ---: | ---: | ---: |" +) + +for (const id of scenarioIds) { + const beforeScenario = beforeScenarios.get(id) + const afterScenario = afterScenarios.get(id) + if ( + beforeScenario?.marginalInstantiations === null && + afterScenario?.marginalInstantiations === null + ) { + continue + } + lines.push( + `| ${markdownLabel(afterScenario ?? beforeScenario)} | ${formatInteger(beforeScenario?.marginalInstantiations)} | ${formatInteger(afterScenario?.marginalInstantiations)} | ${formatDifference(beforeScenario?.marginalInstantiations, afterScenario?.marginalInstantiations)} |` + ) +} + +lines.push( + "", + "
", + "Check times (informational)", + "", + "| Scenario | Base | PR |", + "| --- | ---: | ---: |" +) + +for (const id of scenarioIds) { + const beforeScenario = beforeScenarios.get(id) + const afterScenario = afterScenarios.get(id) + lines.push( + `| ${markdownLabel(afterScenario ?? beforeScenario)} | ${formatTime(beforeScenario?.checkTimeSeconds)} | ${formatTime(afterScenario?.checkTimeSeconds)} |` + ) +} + +lines.push( + "", + "
", + "", + "Type instantiations are the comparison metric. Check time varies with runner load and is informational only." +) + +console.log(lines.join("\n")) diff --git a/scripts/list-examples.mjs b/scripts/list-examples.mjs new file mode 100644 index 0000000..129725e --- /dev/null +++ b/scripts/list-examples.mjs @@ -0,0 +1,38 @@ +import { readdirSync, readFileSync } from "node:fs" +import { resolve } from "node:path" + +const root = resolve(import.meta.dirname, "..") +const examplesRoot = resolve(root, "examples") +const examples = [] + +for (const entry of readdirSync(examplesRoot, { withFileTypes: true })) { + if (!entry.isDirectory()) { + continue + } + + const directory = `examples/${entry.name}` + const packageJsonPath = resolve(root, directory, "package.json") + let packageJson + + try { + packageJson = JSON.parse(readFileSync(packageJsonPath, "utf8")) + } catch (error) { + throw new Error(`${directory} must contain a valid package.json`, { cause: error }) + } + + if (typeof packageJson.scripts?.check !== "string") { + throw new Error(`${directory}/package.json must define a check script`) + } + + examples.push({ + example: entry.name, + directory + }) +} + +if (examples.length === 0) { + throw new Error("At least one package is required in examples/") +} + +examples.sort((left, right) => left.example.localeCompare(right.example)) +console.log(JSON.stringify(examples)) diff --git a/scripts/type-performance.mjs b/scripts/type-performance.mjs index a437869..c238916 100644 --- a/scripts/type-performance.mjs +++ b/scripts/type-performance.mjs @@ -1,7 +1,33 @@ import { spawnSync } from "node:child_process" +import { existsSync } from "node:fs" import { resolve } from "node:path" -const root = resolve(import.meta.dirname, "..") +const options = { + allowMissing: false, + json: false, + root: resolve(import.meta.dirname, "..") +} + +for (let index = 2; index < process.argv.length; index += 1) { + const argument = process.argv[index] + + if (argument === "--allow-missing") { + options.allowMissing = true + } else if (argument === "--json") { + options.json = true + } else if (argument === "--root") { + const root = process.argv[index + 1] + if (root === undefined) { + throw new Error("--root requires a directory") + } + options.root = resolve(root) + index += 1 + } else { + throw new Error(`Unknown argument: ${argument}`) + } +} + +const root = options.root const tsc = resolve(root, "node_modules", "typescript", "bin", "tsc") const scenarios = [ @@ -94,7 +120,15 @@ const version = run(["--version"]) const results = new Map() for (const scenario of scenarios) { - const output = run([...compilerArguments, resolve(root, "perf", "types", scenario.file)]) + const file = resolve(root, "perf", "types", scenario.file) + if (!existsSync(file)) { + if (options.allowMissing) { + continue + } + throw new Error(`Type-performance scenario does not exist: ${file}`) + } + + const output = run([...compilerArguments, file]) results.set(scenario.id, { instantiations: readMetric(output, "Instantiations"), @@ -102,7 +136,9 @@ for (const scenario of scenarios) { }) } -const visibleScenarios = scenarios.filter((scenario) => scenario.hidden !== true) +const visibleScenarios = scenarios.filter( + (scenario) => scenario.hidden !== true && results.has(scenario.id) +) const rows = visibleScenarios.map((scenario) => { const result = results.get(scenario.id) const control = scenario.control === undefined ? undefined : results.get(scenario.control) @@ -116,6 +152,34 @@ const rows = visibleScenarios.map((scenario) => { } }) +if (options.json) { + console.log( + JSON.stringify( + { + schemaVersion: 1, + typescriptVersion: version, + skipLibCheck: true, + scenarios: visibleScenarios.map((scenario) => { + const result = results.get(scenario.id) + const control = scenario.control === undefined ? undefined : results.get(scenario.control) + + return { + id: scenario.id, + label: scenario.label, + instantiations: result.instantiations, + marginalInstantiations: + control === undefined ? null : result.instantiations - control.instantiations, + checkTimeSeconds: result.checkTime + } + }) + }, + null, + 2 + ) + ) + process.exit(0) +} + const widths = { scenario: Math.max("Scenario".length, ...rows.map((row) => row.scenario.length)), instantiations: Math.max("Instantiations".length, ...rows.map((row) => row.instantiations.length)),