Using Solution builder to build project references

[sheetalkamat]

This is based off microsoft/TypeScript#31432

[johnnyreilly]

Very exciting! Looks like there's a compilation error at the moment - I'm guessing we're just waiting for the relevant API to appear in @next 😁

[sheetalkamat]

@johnnyreilly Yes. The PR is almost ready for review and once the build should be able to succeed.

[johnnyreilly]

Cool - let me know when you're ready and we've a passing build and I'll take a look.

I'm guessing the new execution test you've added should pass with the typescript@next test run.

[andrewbranch]

This is looking great! Next week I can try it out on a real life project before we merge. Super excited for this 🎉

[sheetalkamat]

@andrewbranch Can you please try it out now. SolutionBuilder api is in typescript nightly now and I have updated this PR.

[sheetalkamat]

@johnnyreilly This is ready to go in. Please take a look. Thanks.

[johnnyreilly]

Thanks @sheetalkamat! If I get time I'll try to look this weekend.

Am I right in thinking the SolutionBuilder api will be released with TypeScript 3.6?

Also I noticed your new test is being skipped right now:

Skipping test 3.6.0_projectReferencesToBeBuilt as its minimum version of 3.6.0 is greater than our current version of TypeScript: 3.6.0-dev.20190606

https://travis-ci.org/TypeStrong/ts-loader/jobs/542402642

[sheetalkamat]

@johnnyreilly Interesting not sure what I can do to fix that, but I can run that as a single test and it passes.

[andrewbranch]

I was blocked by microsoft/TypeScript#31792 trying to test this in the wild yesterday, but should be able to play with it today.

[andrewbranch]

I’m now pseudo-blocked by #949 (I can observe your changes working, but I don’t get a clean build), but the first thing I noticed was that we don’t create the solution builder if transpileOnly is true. This is how the majority of ts-loaders configure the loader, so we need to do something for them.

When we talked about this before, I think you said that we can’t skip type checking the referenced projects (as transpileOnly would suggest) before we emit them, but I think that’s ok for now.

When #949 is fixed (by #945), I should be able to get a clean build and test the end-to-end experience. I might also have some time Monday to add this to transpileOnly if you’re busy. Thanks!

[johnnyreilly]

It probably makes more sense for @andrewbranch to carry on with the reviewing / merging of this. He's much more knowledgeable about project references than I am.

One question (and it sounds like @andrewbranch is thinking of this too): how does projectReferences support look for users who are using transpileOnly: true and handing off type checking to fork-ts-checker-webpack-plugin?

Does that just work as things stand, or not? I haven't given it a try myself... BTW I'm an admin on that project too and it is also completely open to contributions! ❤️

If he's happy then I'm happy 😀

[artem-malko]

Hi all)
Any news?

[kirill-konshin]

Does this PR address incremental build too?

[andrewbranch]

@kirill-konshin yes (notice that .tsbuildinfo is added to the .gitignore of the execution tests—that’s because the build info file is getting generated 🎉)

[kirill-konshin]

@andrewbranch yes, I just received your comment in incremental-related issue! Perfecto!

[johnnyreilly]

I’ll also be updating the changelog shortly, then I think we can merge this.

Nice work @andrewbranch! Thanks so much!

If you're happy with this do feel free to merge and push out a release (just add an entry to the release to the https://github.com/TypeStrong/ts-loader/releases and the GitHub action should take care of the rest)

[paulvanbrenk]

Thanks @andrewbranch & @sheetalkamat

[andrewbranch]

This is in the process of shipping as v6.1.0. My current thinking is to get some validation that things are generally working, then remove the projectReferences configuration option as a v7.0 if things look good / after any bug fixes from early feedback. I think there should be no reason to toggle project references on or off based on loader config when tsconfig is a perfectly good source of truth, but removing the option will be a breaking change, and I want to be extra sure the functionality is solid before removing the opt-in nature of the feature.

[ericanderson]

Due to random bugs that can happen, leaving the failsafe so regular tsc can use build references but webpack doesn’t have to is good. Maybe 7 could default to on?

[andrewbranch]

I think you’d be better off using a different tsconfig for ts-loader that makes the intention more explicit. In general, I want to move ts-loader away from specialized behavior that contradicts the tsconfig it’s reading. Removing the option from ts-loader doesn’t remove the option for the user, it just shifts it onto TypeScript itself.

[sublimator]

I have a (unfortunately not open source, yet ...) project that is using projectReferences: true, and for some reason webpack (using webpack-cli) stalls and doesn't exit after what seems like a successful build.

Not sure how to provide more info:

    "ts-loader": "^6.1.0",
    "webpack": "^4.39",
    "webpack-cli": "^3.3.8",
    "ts-loader": "^6.1.0",

I happen to be using TypeScript for the webpack config itself, which may or may not indicate something.

[sublimator]

I just cloned this repository and "manually" ran webpack on the test/execution-tests/3.6.0_projectReferencesToBeBuilt and experienced the same hang. Note that I'm on MacOS Mojave yet the same hang is evident on CircleCI running linux inside docker (circleci/node:10.16.2-buster-browsers to be exact)

[sublimator]

Running yarn test from the root of the cloned repo

  projectReferences
    1) should have the correct output
    ✓ should work with transpileOnly (822ms)


  1 passing (4s)
  1 failing

  1) projectReferences
       should have the correct output:

      Uncaught AssertionError [ERR_ASSERTION]: bundle.js is different between actual and expected
      + expected - actual

       exports.lib = {
           one: 1,
           two: 2,
           three: 3
      +    / I am adding this comment here by hand to ensure
      +    / Webpack is using the JS output for project references
       };
       /# sourceURL=webpack://./lib/index.ts?");
       /***/ })
       /******/ });
      
      at compareActualAndExpected (test/comparison-tests/create-and-execute-test.js:467:16)
      at /Users/nicholasdudfield/projects/ts-loader/test/comparison-tests/create-and-execute-test.js:333:13
      at Array.forEach (<anonymous>)
      at compareFiles (test/comparison-tests/create-and-execute-test.js:329:31)
      at Watching.handler (test/comparison-tests/create-and-execute-test.js:194:9)
      at compiler.hooks.done.callAsync (node_modules/webpack/lib/Watching.js:100:9)
      at AsyncSeriesHook.eval [as callAsync] (eval at create (node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:6:1)
      at AsyncSeriesHook.lazyCompileHook (node_modules/webpack/node_modules/tapable/lib/Hook.js:154:20)
      at Watching._done (node_modules/webpack/lib/Watching.js:99:28)
      at compiler.emitRecords.err (node_modules/webpack/lib/Watching.js:73:19)
      at Compiler.emitRecords (node_modules/webpack/lib/Compiler.js:366:39)
      at compiler.emitAssets.err (node_modules/webpack/lib/Watching.js:54:20)
      at hooks.afterEmit.callAsync.err (node_modules/webpack/lib/Compiler.js:352:14)
      at AsyncSeriesHook.eval [as callAsync] (eval at create (node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:6:1)
      at AsyncSeriesHook.lazyCompileHook (node_modules/webpack/node_modules/tapable/lib/Hook.js:154:20)
      at asyncLib.forEach.err (node_modules/webpack/lib/Compiler.js:349:27)
      at done (node_modules/neo-async/async.js:2854:11)
      at /Users/nicholasdudfield/projects/ts-loader/node_modules/neo-async/async.js:2805:7
      at /Users/nicholasdudfield/projects/ts-loader/node_modules/graceful-fs/graceful-fs.js:45:10
      at FSReqWrap.oncomplete (fs.js:141:20)

Likewise yarn test does not seem to exit and stalled after the above output

[johnnyreilly]

@sublimator to do anything useful to help we're going to need a minimum reproduction repo please. If you can make one, raise an issue and link back to this with it that would be amazing.

[sublimator]

@johnnyreilly

Hey, will do!

But as stated above, all you need to repro is clone this repo and run the tests, either with yarn test, or manually invoking webpack on the "fixture", and it hangs

[AndrewKirkovski]

I have identical hanging to @sublimator happening on Windows 10 machine

[johnnyreilly]

Please track this here @AndrewKirkovski:

#998

[maclockard]

Definitely still curious what the expected interaction between ts-loader with projectReferences: true and fork-ts-checker-webpack-plugin is. Will fork-ts need to add project reference support as well?

[johnnyreilly]

I suspect so, yes. If you'd like to raise a PR we'd appreciate it!