The postinstall script can take a while (locally, at least) since it builds the whole project from scratch. I'm not sure to what extent this is mitigated by nx caching in CI, but, locally, the difference betweenSKIP_POSTINSTALL=true yarn andyarn is 9 seconds vs 68 seconds... So I'd think that explains the 1 minute prepare-install time mentioned in#11205

For CI steps that need the tasks done byyarn build, nx's dependency graphshould still execute the required build steps, meaning that we'll mostly just be shifting running time out of the prepare-install step, and into the main CI job. So, likely not a huge overall speedup for those cases. However, for actions thatdon't have any upstream build dependencies, like semantic-breaking-changes-pr, I would expect this to be a significant speedup.

I'd be surprised if this didn't break anything if merged to main, tbh. Previously, many of the nx build dependencies weren't accurately declared, and, therefore, several commands depended on the "build everything" task built intoyarn install. Note that@aryaemami59 has made quite a few fixes in this area, though! So perhaps things will work.

Merging this would also introduce a downside that CI failures would be harder for community contributors to debug since they would generally not know to doSKIP_POSTINSTALL=true yarn && yarn my-failing-command to replicate the behavior in CI.

remove postinstall steps from CI

ba5a5cf

Copy link

Contributor

typescript-eslintbot commentedMay 14, 2025

Thanks for the PR,@kirkwaiblinger!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

Copy link

netlifybot commentedMay 14, 2025•
edited
Loading

✅ Deploy Preview fortypescript-eslint ready!

Name	Link
🔨 Latest commit	`1fb95d6`
🔍 Latest deploy log	https://app.netlify.com/projects/typescript-eslint/deploys/68373c8c836a250008e5ee9d
😎 Deploy Preview	https://deploy-preview-11219--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 97 (no change from production) Accessibility: 100 (no change from production) Best Practices: 100 (no change from production) SEO: 98 (no change from production) PWA: 80 (no change from production) View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to yourNetlify project configuration.

Copy link

nx-cloudbot commentedMay 14, 2025•
edited
Loading

View yourCI Pipeline Execution ↗ for commit1fb95d6.

Command	Status	Duration	Result
`nx test eslint-plugin --coverage=false`	✅ Succeeded	5m	View ↗
`nx run-many -t typecheck`	✅ Succeeded	2m 8s	View ↗
`nx run integration-tests:test`	✅ Succeeded	45s	View ↗
`nx test typescript-estree --coverage=false`	✅ Succeeded	19s	View ↗
`nx run-many -t lint`	✅ Succeeded	14s	View ↗
`nx test eslint-plugin-internal --coverage=false`	✅ Succeeded	6s	View ↗
`nx run generate-configs`	✅ Succeeded	9s	View ↗
`nx run types:build`	✅ Succeeded	6s	View ↗
`Additional runs (27)`	✅ Succeeded	...	View ↗

☁️Nx Cloud last updated this comment at2025-05-28 16:51:08 UTC

kirkwaiblinger added2 commits

May 14, 2025 15:32

add generate-configs requirement

a09d2b7

add repo:lint deps

ea54215

JoshuaKGoldberg previously approved these changes

May 16, 2025

View reviewed changes

Copy link

Member

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

💯 💯yes!

IMO we should merge this ASAP and see what breaks. If I'm understanding the nx philosophy right then all of our scripts should know to run their dependencies first. In a perfect world I think that means we'd be able to eventuallydelete the postinstall script and have it all automagically inferred. 🔪

JoshuaKGoldberg added the 1 approval>=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label

May 16, 2025

bradzacher previously approved these changes

May 16, 2025

View reviewed changes

Copy link

Member

JamesHenry commentedMay 16, 2025

There were some confusing nx config changes just merged in#11135 which have caused conflicts with this one

Copy link

Member

JamesHenry commentedMay 16, 2025

I'll share my thoughts in a loom to try and bring everyone on the same page

Copy link

MemberAuthor

kirkwaiblinger commentedMay 16, 2025

In a perfect world I think that means we'd be able to eventuallydelete the postinstall script and have it all automagically inferred. 🔪

For now, the postinstall script also does a bit of devx setup, in that it installs husky git hooks. So we probably still want it for that? For this PR, I went withSKIP_POSTINSTALL=true instead ofSKIP_POSTINSTALL_BUILD=true (which I periodically use locally) because I figured we don't need husky git hooks in CI.

But yeah possibly the part after theSKIP_POSTINSTALL_BUILD condition maybe could go in the future?

Copy link

Member

JamesHenry commentedMay 18, 2025

The PR and loom are here:#11226

JoshuaKGoldberg removed the 1 approval>=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label

May 26, 2025

Merge branch 'main' into ci-speedup

4e575ae

kirkwaiblinger dismissed stale reviews frombradzacher andJoshuaKGoldberg via4e575ae

May 28, 2025 16:19

Copy link

codecovbot commentedMay 28, 2025•
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.91%. Comparing base(d2ffec7) to head(1fb95d6).

Additional details and impacted files

@@           Coverage Diff           @@##             main   #11219   +/-   ##=======================================  Coverage   90.91%   90.91%           =======================================  Files         501      501             Lines       50869    50869             Branches     8382     8382           =======================================  Hits        46248    46248             Misses       4606     4606             Partials       15       15

Flag	Coverage Δ
unittest	`90.91% <ø> (ø)`

Flags with carried forward coverage won't be shown.Click here to find out more.

🚀 New features to boost your workflow:

❄️Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

kirkwaiblinger added2 commits

May 28, 2025 10:36

linting depends on builds

abe8c0e

use nx task to generate configs

1fb95d6

kirkwaiblinger commented

May 28, 2025

View reviewed changes

.github/workflows/ci.yml

		@@ -78,7 +78,7 @@ jobs:
		uses: ./.github/actions/prepare-install
		with:
		node-version: ${{ env.PRIMARY_NODE_VERSION }}
		- run: yarn generate-configs

Copy link

MemberAuthor

kirkwaiblingerMay 28, 2025

Choose a reason for hiding this comment

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

directly runningyarn generate-configs seems to execute the yarn script directly, ignoring thedependsOn. So, I thinkyarn nx run generate-configs executes the nx task instead, which includes the necessarydependsOn.