Add automated type exports check#2742
Conversation
Co-authored-by: Copilot <copilot@github.com>
📦 Are The Types Wrong? ReportTotal packages checked: 82 ❌ Found issues in 20 package(s):
|
Co-authored-by: Copilot <copilot@github.com>
gingerbenw
force-pushed
the
feature/are-the-types-wrong-check
branch
from
cb5ca60 to
2d3cf84
Compare
|
| Minified | Minfied + Gzipped | |
|---|---|---|
| Before | 48.34 kB |
14.65 kB |
| After | 48.34 kB |
14.65 kB |
| ± | No change | No change |
code coverage diff
Coverage values did not change👌.
Total:
| Lines | Branches | Functions | Statements |
|---|---|---|---|
| 77.38%(+0%) | 69.48%(+0%) | 74.96%(+0%) | 76.43%(+0%) |
gingerbenw
changed the title
There was a problem hiding this comment.
Pull request overview
Adds an automated “Are The Types Wrong?” (ATTW) check to validate TypeScript type exports across all Lerna packages and report results on pull requests.
Changes:
- Add ATTW CLI as a dev dependency and introduce
npm run test:attwto run checks across packages. - Add scripts to run ATTW per package and parse aggregated results into human-readable reports.
- Add a GitHub Actions workflow to run ATTW on PRs and post/update a PR comment with the report.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/run-attw.js |
Runs ATTW across all (non-private) Lerna packages and writes attw-results.json. |
scripts/parse-attw-results.js |
Parses attw-results.json and formats a report (text/markdown/json). |
packages/react-native-cli/package.json |
Adds a types entry pointing to the built CLI .d.ts. |
packages/electron-filestore/package.json |
Adds a files whitelist to control what gets packed/published. |
package.json |
Adds @arethetypeswrong/cli and a test:attw script entry. |
package-lock.json |
Locks dependency tree changes from adding @arethetypeswrong/cli. |
.gitignore |
Ignores the generated attw-results.json artifact. |
.github/workflows/attw-check.yml |
Runs ATTW in CI and posts/updates a PR comment with the report. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| console.log(`Checking ${pkg.name}...`); | ||
| const result = runAttwForPackage(pkg.location); | ||
|
|
||
| if (result) { | ||
| results.push(result); | ||
| checked++; | ||
| } else { | ||
| skipped++; | ||
| } | ||
| } |
There was a problem hiding this comment.
Packages where ATTW fails (or output can’t be parsed) are counted as skipped, but the script can still exit 0 as long as no remaining results have problems. That makes the check non-authoritative (a transient error could hide type issues). Consider failing the run (exit 1) if any non-private package couldn’t be checked, or record failures explicitly and include them in the final exit-code decision.
| - name: Run ATTW check | ||
| continue-on-error: true | ||
| env: | ||
| PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: 1 | ||
| ELECTRON_DISABLE_SANDBOX: 1 | ||
| run: | | ||
| npm run test:attw || true |
There was a problem hiding this comment.
This workflow currently can’t fail the check: continue-on-error: true plus npm run test:attw || true ensures ATTW failures/type issues never fail the job. If the intent is an automated validation check (per PR title/description), remove the error suppression (or add a later step that explicitly fails the job when ATTW reports problems).
| on: [pull_request] | ||
| permissions: | ||
| pull-requests: write | ||
|
|
There was a problem hiding this comment.
The workflow uses github.rest.issues.* APIs to create/update PR comments, but the token permissions only grant pull-requests: write. To avoid 403s, add the required permissions explicitly (typically issues: write, and often contents: read as well). Also consider guarding the commenting step for forked PRs where GITHUB_TOKEN is read-only, so the workflow doesn’t fail unexpectedly.
| }); | ||
|
|
||
| const botComment = comments.find(comment => | ||
| comment.user.type === 'Bot' && |
There was a problem hiding this comment.
comment.user.type === 'Bot' is very broad and could match other automation accounts, leading this workflow to overwrite an unrelated bot’s comment as long as it contains the same header text. Prefer matching on comment.user.login === 'github-actions[bot]' (or a unique marker string in the body) to ensure only this workflow’s own comment is updated.
| comment.user.type === 'Bot' && | |
| comment.user.login === 'github-actions[bot]' && |
| @@ -1,5 +1,6 @@ | |||
| { | |||
| "devDependencies": { | |||
| "@arethetypeswrong/cli": "^0.18.2", | |||
There was a problem hiding this comment.
@arethetypeswrong/cli requires Node >= 20 (per its engines), but this repo’s pinned dev Node version is still 18.x (see .tool-versions). That will make npm ci/npm run test:attw unusable for contributors on the pinned version. Please either bump the repo’s documented/pinned Node version to >=20, or choose an ATTW CLI version that supports Node 18 (and reflect that in CI if needed).
| "@arethetypeswrong/cli": "^0.18.2", | |
| "@arethetypeswrong/cli": "0.17.4", |
| try { | ||
| const output = execSync('npx attw --pack . -f json', { | ||
| cwd: packagePath, | ||
| encoding: 'utf8', | ||
| stdio: ['pipe', 'pipe', 'pipe'] | ||
| }); | ||
|
|
||
| // Parse the JSON output (handle pretty-printed multi-line JSON) | ||
| return JSON.parse(output); | ||
| } catch (error) { | ||
| // attw may exit with non-zero code if problems are found | ||
| // but the output should still be valid JSON | ||
| if (error.stdout) { | ||
| try { | ||
| return JSON.parse(error.stdout); | ||
| } catch { | ||
| console.error(`Failed to parse ATTW output for ${packagePath}:`, error.message); | ||
| return null; | ||
| } | ||
| } | ||
| console.error(`Failed to run ATTW for ${packagePath}:`, error.message); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The try { execSync(...); JSON.parse(output) } catch (error) { ... } block treats JSON parse failures the same as ATTW execution failures. If JSON.parse(output) throws (e.g., ATTW writes any non-JSON to stdout), error.stdout will be undefined and the package gets silently skipped. Split the exec and parse steps so parse errors can log/return the raw stdout, and so genuine ATTW failures vs parse failures are distinguished.
| try { | |
| const output = execSync('npx attw --pack . -f json', { | |
| cwd: packagePath, | |
| encoding: 'utf8', | |
| stdio: ['pipe', 'pipe', 'pipe'] | |
| }); | |
| // Parse the JSON output (handle pretty-printed multi-line JSON) | |
| return JSON.parse(output); | |
| } catch (error) { | |
| // attw may exit with non-zero code if problems are found | |
| // but the output should still be valid JSON | |
| if (error.stdout) { | |
| try { | |
| return JSON.parse(error.stdout); | |
| } catch { | |
| console.error(`Failed to parse ATTW output for ${packagePath}:`, error.message); | |
| return null; | |
| } | |
| } | |
| console.error(`Failed to run ATTW for ${packagePath}:`, error.message); | |
| return null; | |
| } | |
| let output; | |
| try { | |
| output = execSync('npx attw --pack . -f json', { | |
| cwd: packagePath, | |
| encoding: 'utf8', | |
| stdio: ['pipe', 'pipe', 'pipe'] | |
| }); | |
| } catch (error) { | |
| // attw may exit with non-zero code if problems are found | |
| // but the output should still be valid JSON | |
| if (error.stdout) { | |
| try { | |
| return JSON.parse(error.stdout); | |
| } catch (parseError) { | |
| console.error(`Failed to parse ATTW output for ${packagePath}:`, parseError.message); | |
| console.error('Raw ATTW stdout:', error.stdout); | |
| return null; | |
| } | |
| } | |
| console.error(`Failed to run ATTW for ${packagePath}:`, error.message); | |
| return null; | |
| } | |
| try { | |
| // Parse the JSON output (handle pretty-printed multi-line JSON) | |
| return JSON.parse(output); | |
| } catch (parseError) { | |
| console.error(`Failed to parse ATTW output for ${packagePath}:`, parseError.message); | |
| console.error('Raw ATTW stdout:', output); | |
| return null; | |
| } |
Goal
Add an automated check using are the types wrong to validate type exports for each package.
You can also run
npx lerna exec -- attw --pack .in the root of the repo to see a per-package breakdown