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

Add a monorepo (project references) support for via the --build flag #19

Merged
merged 4 commits into from
Mar 7, 2024

Conversation

iamdoron
Copy link
Contributor

@iamdoron iamdoron commented Mar 3, 2024

This PR detects when a typescript project uses project references. If that's the case, it will use the "--build" flag, which will properly enable incremental builds.
note that the current solution works on a monorepo, but it's not using the incremental build

for example, if I go on my machine to fixtures/monorepo/package2 repeatedly calling

$ npx borp

The TypeScript compilation completes around 300ms
While using this version, it completes around 100ms (=> properly using incremental build)

I also fixed a small bug in in the watch mode - it would report "TypeScript compilation complete" with invalid time lengths

Copy link
Owner

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@iamdoron
Copy link
Contributor Author

iamdoron commented Mar 3, 2024

@mcollina there was an issue with the tsconfig for package1 in the fixture.. for some reason it works on Mac but not ubuntu. if you don't specify outDir in monorepo/package1/tsconfig.json, it is being extended via monorepo/tsconfig.base.json with outDir 'dist'. but then it creates the dist of package1 in monorepo/dist 😅
I think I fixed it, but need to run the CI again to check

@iamdoron
Copy link
Contributor Author

iamdoron commented Mar 4, 2024

@mcollina the issue on node 21 on windows looks related to nodejs/node#51766
last CI that passed used 21.6.1 and the issues started at 21.6.2
if you want we can fix the node 21.x to 21.6.1. Not sure what's the best way to get this merged 😄

@mcollina
Copy link
Owner

mcollina commented Mar 4, 2024

@iamdoron
Copy link
Contributor Author

iamdoron commented Mar 4, 2024

yes, there is a problem with the reporter. but even if fixed, it would still fail with EPERM, no?

@mcollina
Copy link
Owner

mcollina commented Mar 4, 2024

unfortunately yes. I think you can drop v21/windows from the test matrix, then we'll re-add once the node core bug is fixed.

@mcollina mcollina merged commit 15061f7 into mcollina:main Mar 7, 2024
5 checks passed
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.

2 participants