tsloader's project reference support seems to broken

[desmap]

Expected Behaviour

Project references compile/transpile way too slow, this can be tested and reproduced, I put extensive benchmarking below. transpileOnly: true doesn't make any difference on ref compile times so there must be something wrong, more details:

1. tsloader compiles refs slower than tsc- -b -w (no news, but just as a reference for the next points!)
2. tsloader+transpileOnly transpiles refs exactly as slow as tsloader omitting transpileOnly (compare BROKEN 1 with BASE)
3. compare BROKEN 2 also to the babel+tsc-noEmit version (OPTION 3) which shares the same architecture and refs are transpiled ultra fast
4. tsloader seems to affect also ts-fork-checker perf negatively: Slow Performance on Incremental Runs (Project References) fork-ts-checker-webpack-plugin#463 (comment)

tldr ATM it's recommended to go for babel-loader for fast project reference transpilation and a dedicated tsc -b -w --noEmit process for type checking until tsloader's bug is fixed

CC @johnnyreilly, @sheetalkamat, @piotr-oles, @appzuka

thanks to r/keiser_sozze for hinting me to babel+tsc noEmit.

Here again the updated comparison table, pls focus on OPTION 3 and BROKEN 1 and 2 and there, the bold figures highlight the broken transpilations:

setup	type-checking?	initial build w/o changes before (warm start)	initial build w/ changes before	initial build after rm -r dist (cold start)	rebuild on 1st change in non-reference	rebuild on 1st change in reference	rebuild on 2nd change in non-reference	rebuild on 2nd change in reference
OPTION 1 tsc -b -w + webpack-dev-server just bundling tsc's output, aggregateTimeout: 0	✔	1.5s	13s	14.7s[2]	2.3s	3.4s	2.2s	2.4s
webpack-dev-server+tsloader, fork-ts-checker	✔	5.4s	5.2s	9.6s	1.8s	4.5s	1.1s	3.3s
BASE webpack-dev-server+tsloader	✔	-	-	9.5s	2.1s	4.2s	1.9s	4.4s
BROKEN 1 webpack-dev-server+tsloader, transpileOnly		-	-	8s	1s	4.4s	0.4s	3.7s
BROKEN 2 webpack-dev-server+tsloader, transpileOnly, tsc noEmit	✔	5.2s	5s	12.2s	1s	6s	0.9s	6s
OPTION 3 benchmark winner, webpack-dev-server+babel, tsc noEmit	✔	6.7s	6.8s	6.6s	1.7s	1s	0.3s	1s
BROKEN 3 webpack-dev-server+tsloader, transpileOnly, fork-ts-checker	✔	4.7s	4.5s	9.9s	1.2s	5.5s	0.9s	6s
just tsc -b -w without bundling	✔	6.6s	11.5s	13.2	1.9s	2.9s	1.67s	1.94s
just webpack-dev-server bundling tsc's output, aggregateTimeout: 750		-	-	1.56s	0.5s	0.6s	0.4s	0.3s
just webpack-dev-server bundling tsc's output, aggregateTimeout: 0		-	-	1.5s	0.4s	0.5s	0.5s	0.4s
webpack-dev-server+tsloader w/ happyPackMode, fork-ts-checker w/semantic+syntatic, threadloader w/infinite pool	✔	-[1]	-	-	-	-	-	-
webpack-dev-server+ts-loader, tO true, import dists	yes	3.7s	4s	9.2s	1.3s	6.8s	0.7s	5.3s
tsc -b -w w/o bundling, import dists	yes	6.3s	6.61	6.6s	1s	3.6s	0.9s	2.26s
webpack+ts-loader, tO true, fork-ts, import dists	yes	3.4s	3.4s	8.8s	3.3s	8.9s	3.4s	8.6s
tsc -b w/o bundling, import dists	yes	6.5s	6.79s	14.4s	6.6s	4.29	6.96s	6.16s

Actual Behaviour

BROKEN 1 should be much faster than BASE

Steps to Reproduce the Problem

git clone https://github.com/RyanCavanaugh/project-references-demo, take your preferred webpack config and play around with the settings in the benchmark table.

Location of a Minimal Repository that Demonstrates the Issue.

https://github.com/RyanCavanaugh/project-references-demo + settings from the table

[johnnyreilly]

Thanks for your detailed benchmarking @desmap. If you'd like to look into submitting a PR to resolve this we'll happily take a look.

[desmap]

If you'd like to look into submitting a PR to resolve this we'll happily take a look.

Are you serious?

I don't know the code base, I spent now 48h in benchmarking your lib and asking me for a PR is—no offense—borderline.

I could offer you to make a PR on the docs which states 'project ref support is highly experimental', so other users don't fall in the same rabbit hole like I did. It's your lib and you hurt your and fork-ts-checker's users with keeping this bug alive. It's your call and I wish you all the best!

[johnnyreilly]

@desmap this is open source. No-one is paying me to build ts-loader, I do it in my spare time with the help of all the generous people who contribute to the project, and have contributed in the years it's been running. This is a generousity game we're all playing.

The lib is not perfect but it's very useful to many people. You are not entitled to anything, but you are welcome to join in.

Perhaps you could help make ts-loader even better and I invite you to do so ❤️🌻

[appzuka]

@desmap, I forked the demo repo as you suggested and added webpack.config.js to test the various options. In the readme I have detailed what I found. I won't repeat that all here but in summary, ts-loader, project references, transpileOnly & ForkTsCheckerWebpackPlugin are all working as expected. I am not seeing a bug in ts-loader here. You can see the repo at:

https://github.com/appzuka/project-references-demo

It is certainly possible that with larger projects we will see a significant performance slowdown. I will try to expand the repo to test this although I have another project using all of ts-loader, project references, transpileOnly & ForkTsCheckerWebpackPlugin and it works fine (initial build is around 30 seconds).

Hopefully, you can see where this code differs from yours and understand why you are seeing something different. If you can share a repo showing the behaviour you benchmarked I'll take a look.

[desmap]

@appzuka nice that you took the effort and take this seriously, expected and appreciated.

To bloat the files, you have to add something to all files and import external modules, otherwise you won't see a difference.

I've put following 1,000 arrow functions in an array (the extraError is just to add type errors if needed, otherwise it doesn't have any relevance) in every file. I also imported react, react-dom, reflect-metadata and type-graphql's browser-shim to bloat even more.

const someArray = [
  (param: string) => {
    const extraError = param
    console.log(extraError)
  },
  (param: string) => {
    const extraError = param
    console.log(extraError)
  },
  ...
  ...
]

So after you bloated the files you have to compare BROKEN 1 and BASE.

BROKEN 1's ref compiles on ref changes must be faster than BASE because of transpileOnly but they aren't.

To create even more drama just run your tests with babel-loader and you will be shocked how much faster babel is and project reference worked out of the box without any setup with babel-loader (however, there is even a dedicated ms plugin).

Again: Important are only the compile times on ref changes. The rest (initial compile, compile on non-ref change) is nice-to-have but I would focus just on the ref changes to isolate the cause better. The rest looks ok to me, however initial compile should be also faster in BROKEN 1 (they are indeed a bit faster but this could be also vps tolerance) but again, just focus on ref changes for the sake of simplicity.

I am not seeing a bug in ts-loader here.

I respectfully and highly doubt this. Pls bloat your files, do proper time measurements on ref changes and but also non-ref changes, also for a babel setup and share quantitative data. Otherwise it's more difficult to follow.

I'd like to help you guys wherever I can but also I am limited on time/have to move on and my peers already think I am nuts doing such pedantic time measurements but it was worth, I learned a lot and with babel-loader I now save 5 (!) sec on avg per ref compile and still have perfect and async type checks. And most of my code base are refs. Again, I can't stress this enough, refs are the very best feature of TS and allow truly amazing monorepos, so yeah. I am super happy to have now everything at amazing compile times. To push babel even further (than in OPTION 3) just exclude node_modules.

Finally a comment I got from that guy on reddit who hinted me to babel, pls don't see this as a rant or offense, just as valuable feedback and an opportunity to improve (more options are always good and hope to see ts-loader back in the ring soon again):

I've been using TS for 3+ years and since babel v7.0 (with ts support) released, ts-loader, ts-jest etc. never performed as good as tsc+babel. Microsoft officially promotes the babel approach in some of its sub-projects and internal projects [...] people coming to the TS community are mislead towards [ts-loader, ts-jest, etc]. Wish microsoft/typescript or webpack/webpack were more vocal and opiniated about the topic.

Many people try to show babel as a monster, but in the long term, it's much simpler to use than ts-*-loader, ts-jest etc and less hassle. And preset-env with auto polyfills is really dope. I have no idea how people live without it.

Wish you guys a nice weekend and keep pushing!

[appzuka]

@desmap, I'll bloat the source files and report back.

Please confirm your setup using babel-loader. I assume you are using @babel/preset-typescript, but that does not support building project references - you would need to build the references separately.

ts-loader does build project references but transpileOnly will not affect the way project references are built. Project references will still be built with syntax checking. Therefore I would not expect your Broken1 settings to be faster than Base.

[desmap]

Please confirm your setup using babel-loader.

Yep. I use babel-loader and it compiles refs out of the box.

There is also https://github.com/microsoft/webpack-project-references-alias but I didnt need it, think it's if you want to import the refs module-style but IDK

Here is the config:
modules.rules in webpack.config.js

 {
   test: /\.tsx?$/,
   use: [
     {
       loader: 'babel-loader',
       options: {
         rootMode: 'upward',
       },
     },
   ],
 },

babel.config.json

{
  "presets": [
    "@babel/preset-env",
    "@babel/preset-react",
    "@babel/preset-typescript"
  ],
  "plugins": [
    "babel-plugin-transform-typescript-metadata",
    ["@babel/plugin-proposal-decorators", { "legacy": true }],
    ["@babel/plugin-proposal-class-properties"],
    "babel-plugin-parameter-decorator"
  ]
}

transpileOnly will not affect the way project references are built. Project references will still be built with syntax checking.

Ok, is this caused by tsc or ts-loader? And if former why is then even the pure tsc -b -w variant OPTION 1 faster with ref compiles (one row below BROKEN 3) ?

Edited: I mean OPTION 1 in the last sentence

[desmap]

i remember an issue on the ts issues where users wanted to have noEmit to work with tsc -b or incremental builds and this wasn't possible because tsc -b need d.ts files but then the ts team offered emitDeclarationOnly and also intro'd composite false. I am not sure if tsc needs even with latter still a compile step for refs. If yes then we found the culprit and it would make sense that fork-ts cannot fork because yeah, this can't be parallelized, or can it? so ts-loader would be innocent but idk, this is getting complicated 😉

still OPTION 1 shouldn't be faster in any case

[desmap]

fwiw I run the dedicated type check process in OPTION 3 with

tsc -w --incremental --declaration --emitDeclarationOnly 

[appzuka]

@desmap, I used your babel setup but still cannot see it build project references. You can repeat my findings using the 'babel' branch of my repo:

git clone https://github.com/appzuka/project-references-demo.git
cd project-references-demo
git checkout babel
yarn install
yarn build <- Module not found: Error: Can't resolve '../lib/zoo/zoo'
tsc -b
yarn build <- OK

This discussion indicates that this is normal for babel-loader - it simply strips off types similar to ts-loader's transpileOnly mode:

microsoft/TypeScript-Babel-Starter#35

If this is correct it would explain why your benchmark winner only takes 1 second after a reference change - it is not rebuilding the reference. Even tsc -b -w takes 2-3 seconds.

[desmap]

tldr babel builds refs, there were some config/structural errors in your repo

here you go: https://github.com/desmap/project-references-demo/tree/babel just compare with Github's compare feature the babel branch with yours; there are more changes bc of code formatting etc

to get the babel compile run do npm run build-another-package and run it with node dist/another-package/main.js but first read the following.

i changed a few things just to give a better understanding; the project-reference-demo structure is ok to grok project references but it's not perfect (a dedicated packages folder would make it easier to understand but yeah) and naming is here and there odd:

• i renamed src to "another-package"; using src in project reference structures is not so common (what I see); src folders create a huge complexity in later resolution declarations, no need for that extra headache; basically all folders in your monorepo's "packages" folder are sources and don't need any dedicated src folder; in this repo Ryan, the original author, just omitted the packages folder and put the packages right in the root
• then the output is a sibling folder to the packages folder (which in our case does not exist, so it's a sibling of the packages directly) and also reflects the structure of the (imaginary) packages folder; Ryan named this folder 'lib' but we change this...
• i renamed the lib folder to dist to make more clear that it contains output (it also serves as a lib for packages outside of project references and babel, see below, so lib as a name could make sense but here i found it rather confusing)
• there're some path-related config errors, so that were the reasons that it didn't work, eg Ryan named zoo's index zoo/zoo.ts and not zoo/index.ts, hence you need to require('../zoo/zoo')
• be aware that we took out another-package, which will be compiled with webpack, out of the solutions file (the root tsconfig not base) to not let tsc double compile the webpack stuff (iirc it wasnt in the file anyway)

if you want to compile backend related packages you do run npm run build-the-rest which employs tsc -b; if you want to compile frontend related matters, you use webpack and in our case npm run build-another-package which uses babel and tsc for type checking

this is just an example structure, you could also pipe webpack stuff into a public folder which gives a cleaner architecture. It's important to understand that you now have two build pipelines coexisting neatly.

The beauty is now if you have some lib which doesn't get along at all with project references (like Nextjs): you could still add a Next project as a sibling and import any package from dist/ with eg tsconfig path or baseUrl: '..'

final note: i am thinking also about to setup the same fast compiles for backend matters but webpack is frontend and IDK if this would add more complexity than benefits; but it would be worth a try because this setup is so fast; but it would suffer drawbacks like no option to let others like Next import single composite packages because webpack creates one big file and you don't want to setup for every tiny component/package a webpack config/build step but just for front-facing apps.

ps there will be a type error or warning which is ok, so we see that type checking is working

[appzuka]

@desmap, thanks for sharing the repo.

I can see that you are not actually using project references in your front end build. In your backend build you execute tsc -b, which builds the entire project. After this you see that the dist folder gets populated with all the transpiled code.

For the front end, when you run yarn build-another-package you are running 2 commands:

webpack
tsc --incremental --declaration --emitDeclarationOnly --preserveWatchOutput

The first builds the project with webpack and babel-loader. Babel-loader just strips out the types, it does not check for syntax errors (there are other aspects of Typescript it does not handle either, such as const enums). For this reason, you run the second command, which should report Typescript syntax errors.

Both of these commands run, and a bundle is produced in dist, but neither use project references and the project references are not compiled. Run yarn build-another-package after running yarn clean and you will see that only the final bundle is in dist, not the compiled references.

So to be clear if anyone comes across this thread, babel-loader does not understand project references. You will notice that babel-loader produces a warning "export 'Dog' was not found in './dog'. This is because Dog is an exported Typescript interface. Babel-loader simply strips this out because it does not understand it and then reports that it is missing. If you use ts-loader you will not get this warning.

Webpack with babel-loader does build the project, even if you have not compiled the references. This is because you are building from the typescript source files, not the compiled references. In your another-package/index.ts you import from the zoo typescript source:

import { createZoo } from '../zoo/zoo'  // imports from zoo.ts

If you are using project references and you want to benefit from using the precompiled projects you should be importing from the compiled javascript:

import { createZoo } from '../dist/zoo/zoo' // imports from zoo.js

This gives you the benefit of using the pre-compiled files when building your project, which will reduce the warm-start time. It does mean that if you make a change to a reference you will need to re-build the reference. You would need to be running tsc -b -w in a separate shell and then you would need to add the 2-3 seconds it takes tsc to build the reference to the 1s it takes webpack to bundle the project.

In your benchmarks, you see essentially the same time to rebuild after a change in references and non-references for your Option3. This is because Option3 does not use references so it is doing the same thing. The settings with ts-loader (your Broken2) takes the same time for non-references but much longer for changes to a reference. This is because you are asking ts-loader to do everything that babel-loader does and then build the references, even though they are not used. If you set ts-loader to run with transpileOnly: true and with projectReferences: false I expect it would perform the same as babel-loader with better handling of typescript.

I don't believe it is fair to say that there were some config/structural errors in my repo. We have made different choices and those choices make different performance tradeoffs. If rebuild time is most critical to you then perhaps your configuration is best for you. The babel-loader vs ts-loader is not the relevant thing to look at here - they both have fast rebuild times. The issue here is comparing webpack in watch mode vs tsc -b -w. Your setup uses project references for backend build but not for frontend. They share a single typescript codebase so I don't think there is a problem with that. You gain fast development rebuilds at the expense of a slower development start time. If you are happy with that tradeoff that is fine. Others may have different priorities and choose to go a different way.

I did bloat the codebase as you suggested and found ts-loader is behaving exactly as expected. Its performance with project references is the same as running tsc -b -w in a separate process. Having project references integrated into ts-loader means you can build your project with a single webpack command rather than managing a 2-stage flow.

Your benchmarks have uncovered some interesting points about different ways to configure projects. I don't believe they show that there is a bug in ts-loader or that ts-loader's performance is worse than alternative methods.

It is also clear that using project references with webpack is not straightforward and documentation is badly needed. I intend to add some documentation and examples to ts-loader so that others can get the most of out this great feature which has been added to tsc.

[desmap]

tldr I like your thoughts, tested and benchmarked them but (i think) babel-loader still excels at perf and ux

@appzuka thanks and to summarize your thoughts:

(1) tsc, (2) webpack+ts-loader, (3) webpack+babel-loader all create different kind of builds and we should consider this when benchmarking, i totally agree:

1. builds ALL refs declared in the roots solution file as dedicated files
2. builds entry and entry's direct and indirect imports as dedicated files
3. builds only entry as a dedicated file

hence you are right, 3 needs a dedicated tsc -b to build the entire project ref structure but even 2 might need a dedicated tsc -b to compile not imported refs of a project ref structure, e.g. pure backend refs. 2 definitely builds more than 3 but not all refs. I just tested this myself: ts-loader with projectReferences: true does not build not imported refs (refs = all refs in the solution file).

hence...

1. tsc -b understands and compiles ALL project refs as dedicated files
2. ts-loader understands but compiles only imported project refs as dedicated files
3. babel-loader does understand project refs but only compiles entry

so babel-loader understands project refs, creates proper builds which run but does not compile all project refs, tsloader neither. besides, that babel-loader strips away types is intended and for any ts-related matter we have the dedicated tsc type checking process.

to reframe aforementioned:

• for every terminal frontend package we have webpack+babel-loader+parallel tsc --incremental processes
• for every frontend component package we have just frontend ts sources which can be imported
• for every backend and/or generic package we have ts sources and tsc -b builds which can be imported by internal refs and external projects

A benefit on this setup is that if I just work on frontend stuff and don't need to get refs to be compiled as dedicated files but just need my main.js bundle I dont face any overhead of checking/recompiling potentially affected refs.

then you stress that dist js files should be imported and not the ts sources. i haven't seen this in the wild, even Ryan's reference demo repo imports ts sources. however, i think that your thought is actually quite good, so i benchmarked it.

i changed in my bloated 1,000 function repo from this structure...

package1
package2
package3
dist
  package1
  package2
  package3

to this...

package1
  dist
package2
  dist
package3
  dist

and changed the imports from eg ../p1 to ../p1/dist

you find the benchmark results in the og post at the top in the last rows. first for tsloader and then pure tsc -b.

so if we look at the figures, they don't make sense. what you wrote (that taking precompiled js files should be faster) makes sense but is unfortunately not reflected by the benchmarks. I edited this because you actually wrote that warm-up is faster which is true but the ref compile times are even bigger now.

since the warmup times are just slightly faster and the rest not, i assume that tsc -b or --incremental knows what have been compiled also with importing ts sources and does not recompile them (hint: tsbuildinfo), so I still believe importing ts sources is good too. i like also the import ux more with a sibling dist folder. but i might be wrong and maybe someone from the ts team knows more and can chime in here.

in general, i think it's nice that ts-loader compiles all its imported packages as dedicated files (better than babel-loader) but i still need a dedicated tsc -b process for all refs. so if i need anyway a dedicated tsc -b process, i prefer really fast builds for webpack frontend matters and then we talk again about babel-loader.[1]

[1] in a full dev setup you would have three processes: webpack+babel-loader with parallel tsc -w --incremental type-checking, another parallel tsc -b -w for the rest

Edit: I tested again last two rows in watch mode to just stay consistent (which two new rows before, so you have now 4 new rows). Now, definitely warm-up is faster (so what you wrote) but the ref compiles not so much compared to the ts import variants, I also edited the respective parts of the text

[appzuka]

@desmap, I think if you have a solution which you are comfortable with you should go for it.

This issue started with a report that ts-loader's project reference support is broken. I have not seen any confirmation of this. The rows named 'Broken' are not comparing like with like. If you still believe there is a problem I suggest you share a repo which demonstrates this.

I created a repo with 100 files each with 100 blocks of code to transpile and I saw that ts-loader with transpileOnly is as fast as babel-loader and ts-loader with projectReferences is as fast as running tsc -b in a separate process.

[desmap]

project reference support is broken. I have not seen any confirmation of this.

pls check top post: tsloader+transpileOnly transpiles refs exactly as slow as tsloader omitting transpileOnly (compare BROKEN 1 with BASE). here we could debate (again) if we blame tsloader or tsc.

then we see that even OPTION 1's (pure tsc -b -w+webpack) ref times are faster than BROKEN 1 which shouldn't be the case because tsloader uses tsc under the hood, so why is it slower (ref times!) than tsc+webpack? and OPTION 1 builds all refs while BROKEN 1 builds only the imported ones?

are not comparing like with like

re babel yes, re BROKEN 1 vs BASE and BROKEN 1 vs OPTION 1 (ref times!) no, compare aforementioned

I saw that ts-loader with transpileOnly is as fast as babel-loader and ts-loader with projectReferences is as fast as running tsc -b in a separate process.

Would have been helpful if you shared quantitative data such as time measurements for respective setup and build variants.

I think if you have a solution which you are comfortable with you should go for it.

Nice closure, I think the same. We spent way too much time here and should do some real work now, both with their preferred setup. 😉 Wish you all the best and I enjoyed our conversation.

[appzuka]

I submitted a PR to for ts-loader's docs to make it clear that transpileOnly has no effect on project references.

The code I used to confirm ts-loader is running correctly on a large codebase is available in the benchmark branch of the repo I shared:

https://github.com/appzuka/project-references-demo/tree/benchmark

The steps to run the benchmarks and the times I obtained on my system (WSL2, i7 3.4GHz) are in the readme.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

Closing as stale. Please reopen if you'd like to work on this further.