-
-
Notifications
You must be signed in to change notification settings - Fork 431
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
Support for TS 3 project references #815
Comments
Hey @cactucs, Project references support isn't implemented as yet. If you'd like to we'd be happy to look at a PR. If you're wondering what would need to change then take a look at the gulp-typescript changes as a guide: https://github.com/ivogabe/gulp-typescript/pull/579/files Don't worry about broken tests; feel free to just start the PR process and we can use that as a basis for collaboration 😄 |
@johnnyreilly I started taking a look at this; I got curious about it at the exact same time as @cactucs. It seems that ts-loader’s As a test, I wrote a project where the root tsconfig file uses I'll fork and push what I've done so far. |
That said, I'm not sure what exactly the expected behavior for webpack and project references is. |
First of all, thanks for your work @andrewbranch!
An excellent question; and honestly I'm not sure I know the answer. I'll cc @RyanCavanaugh @rbuckton in case they have a view. But for myself I'm not too sure. That said, I'm open to exploring options. |
Just pushed a commit that “fixes” making the language service aware of project references, causing an error to occur when the dependent composite project isn’t built (which is indeed what
but rather the semantic diagnostic generated by the program, which you see with tsc:
As I'm beginning to understand more how this works, it seems like ts-loader could take one (or either/both with a loader option) of two positions:
|
Please keep investigating. I'm open to having an option that flips between modes if that seems useful. Probably best you do some real world experiments and report back on what seems to make sense. |
Sounds good to me; I'm happy to keep experimenting, but would definitely appreciate input from the TS team here. Technically I guess they’re coworkers of mine so maybe I can track someone down. 😄 Pushed another commit that basically finishes option 1 for the language service path; should hopefully be simple enough to do the same for the I've been testing and debugging ad-hoc for this so far; the test directory here is daunting. Any tips on where to start if I were to begin writing some real tests? Edit: also, do you have a preference on how to do TypeScript version detection? We can only call |
Great! @mhegazy @DanielRosenwasser @sheetalkamat @RyanCavanaugh @rbuckton would any one of you be able to advise your erstwhile colleague @andrewbranch on what a sensible course of action here would look like?
Yeah it is a bit. Essentially there's 2 test suites; each with a subtly different purpose. The comparison test pack and the execution test pack. Have a read of the README's to find out more about them: https://github.com/TypeStrong/ts-loader/blob/master/test/comparison-tests/README.md The TL;DR is this: the comparison test pack captures file outputs for future diffing. It's useful for checking files are generated in response to various actions (you have fake files being updated and capture the resulting output). The execution test pack actually executes tests written in TypeScript, and transpiled into JS. These tests are more reliable but don't provide any insight as to what triggers file generation. Probably given your use case a comparison test will be most appropriate. But I know these are daunting and I'm happy to pitch in and help guide you.
Probably just doing cc @TheLarkInn |
@johnnyreilly, how would you feel about splitting my proposed two possibilities into two PRs? I think I’m almost done with the first part (using built project references, but not building them), which I think is useful on its own, and I would hate to see it languish if figuring out the second part takes a long time or I can’t commit as much time in the future. I'll go ahead and open a WIP PR and start trying to figure out the tests. |
I’ve hit a bit of a speed bump upon realizing that |
This is true unless they've set So you should be able to derive the location of the original source file using the info in the |
Hey all, thanks for taking this up. Here to answer some questions (I'll be checking in on this all day so let's get chatty). To compute the output JS filename, you can use this code from function getOutputJavaScriptFileName(inputFileName: string, configFile: ParsedCommandLine) {
const relativePath = getRelativePathFromDirectory(rootDirOfOptions(configFile.options, configFile.options.configFilePath!), inputFileName, /*ignoreCase*/ true);
const outputPath = resolvePath(configFile.options.outDir || getDirectoryPath(configFile.options.configFilePath!), relativePath);
const newExtension = fileExtensionIs(inputFileName, Extension.Json) ? Extension.Json :
fileExtensionIs(inputFileName, Extension.Tsx) && configFile.options.jsx === JsxEmit.Preserve ? Extension.Jsx : Extension.Js;
return changeExtension(outputPath, newExtension);
} Some general discussion - in a way, project references and tsbuild's behavior of building upstream projects are actually totally orthogonal, though obviously related. Project references are just an advanced form of path mapping, and tsbuild's build ordering/dependency algorithm is selected mostly for simplicity. The choice of building referenced projects or not, and the choice of what the right up-to-date check, really depends on the tool and I'm honestly not well-informed enough about Webpack to make the right call on whether there's one right answer or space for configurability here. |
@RyanCavanaugh awesome, thanks! Appreciate you taking the time to offer some expertise here. One immediate question: looking at the snippet from |
@andrewbranch certainly. If you can come up with a list we can open a bug on our side and review them |
I think @johnnyreilly this is making me realize, though... if the referenced project uses EDIT: alternatively, we could choose not to support |
Yeah - I don't think this works in the scenario with ts-loader generally tries to be a drop-in replacement for |
@RyanCavanaugh the only other thing that's a little awkward at the moment that maybe you can help with: determining the project reference (if one exists) for a given file path. Right now I'm doing program.getProjectReferences().find(ref => ref.commandLine.fileNames.includes(fileName)) Is there a more efficient way, whether public or internal, for getting that? EDIT: aside from getting the JS output file, we’ll also need to get the source map output if it exists, although that one I assume is always just swapping the extension on the JS output filename—right? |
@andrewbranch I believe that's the simplest way to do it, yes. I don't think we provide any API for it
Correct; there are currently no compiler options which move the .map file |
It seems the presence of the I'm using
|
@pkieltyka You've probably found this already, but just in case: you're hitting microsoft/TypeScript#26554, which you can work around without breaking |
thank you @simonbuchan — thats a nice trick to know. I ended up making a separate Will ts-loader be able to support project references soon? this would be helpful in a monorepo situation, something we're doing in our setup already with ts-node on a server project (and works very well) |
@pkieltyka I'm hoping to get #817 merged next week. Note that means you'll still need to run |
@andrewbranch thats awesome, perhaps trigger |
FWIW I'm a maintainer of https://github.com/Realytics/fork-ts-checker-webpack-plugin also and I'd be happy to review PRs to that as well 😊 |
I'm having an issue with this option enabled, where it appears ts-loader attempts to load the references even though I have projectReferences set to true. My structure:
I attempt to build via an npm script ie Anybody, especially @andrewbranch , have an idea why this might be causing me problems with this new projectReferences feature? It seems to me that ts-loader would ignore the ts in the sharedLibs and use the (now built and up-to-date) js files in that same location instead. Edit: Additional detail of error:
|
Hey @mscottnelson, would you be able to link to your project or an example that reproduces the issue? I'd have to take a look at what's in each tsconfig. |
Hi @andrewbranch , many thanks for the rapid response. I suspect this is happening because the path references are not migrated / updated to point to the new relative locations when Webpack produces its output directory/file. Not sure what the most idiomatically correct solution would be. Here's an example that reproduces the issue: |
Hey @mscottnelson! Just did some debugging, and figured out that you are hitting the exact same thing that I hit here: #817 (comment). Specifically, when I follow Webpack’s request for The weird thing is, though, that At the moment, if you add a By the way, for anyone interested in looking into this, or other ts-loader issues, I quickly debugged with
then put a breakpoint in |
Could it be because your host doesn't have readDirectory implemented correctly? |
@sheetalkamat hmm, my PR didn't change that, but you would certainly know better than I! 😄 It looks pretty innocuous here: Line 124 in f945f4f
Would you expect something different? |
Ok, so if I check the resolved project reference object more closely, it has an error in
So there is a default So in fact, when I said
It looks like I was wrong. Maybe only |
@sheetalkamat you were exactly right! Well, almost—I don't know if I would call it my host—it looks like TypeScript was creating one for me, and it didn't put And @mscottnelson, sure enough, if you upgrade your example project to |
Boom! Does this mean that project references should start to work as expected when 3.1 ships? I'm assuming this will ship with 3.1: |
@johnnyreilly the tag says 3.1.1, but seeing as 3.1 is currently in RC, I think it's all one and the same? I'm opening a PR now to add a note to the README. Do you want to add a note to your blog post? |
Good shout - just waking up over here. Will try and do that later today. Good investigation! BTW I might take your debugging tips and document them somewhere; I've always relied on logging to diagnose issues with ts-loader thus far. Being able to use Code to properly debug is 💯 times better! My kingdom for a debugger 😄 |
I've had break points in original source working before, though I use Webstorm. You could try ndb? |
I might take it for a whirl at some point. I use both Chrome and Code to debug in other contexts and find both tremendous. |
Many thanks for the help. Updating to TypeScript 3.1.1 with |
Small status update: I spent an hour or so this morning exploring the current state of the tsbuild API to see if it contains what we need for ts-loader to build/watch upstream projects. It mostly does, but currently the API isn't finalized and the whole thing is marked internal: https://github.com/Microsoft/TypeScript/blob/85a3475df87b9c10240570f32090970262c99b54/src/compiler/tsbuild.ts#L1-L2 I think ts-loader’s full support for project references will have to wait until that’s finalized, lest we get broken by minor or patch TypeScript releases. @sheetalkamat, two things:
const project = path.resolve('./tsconfig.json'); // “Root” project, has dependencies
const builder = ts.createSolutionBuilder(host, [project], {});
const { buildQueue } = builder.getBuildGraph([project]);
// buildQueue is an array of paths to tsconfig files, in the order that they need to be built.
// That means our “root” project is the last element in the array. We want to build every
// project _except_ that one.
const projectsIWantToBuild = buildQueue.slice(0, buildQueue.length - 1);
const dependencyBuilder = ts.createSolutionBuilder(host, projectsIWantToBuild, {});
dependencyBuilder.buildAllProjects(); I realize all this is still in flux, so I just wanted to give you an idea of the functionality we’ll need here, since it’s not the typical case of running a solution build programmatically. At the moment, it feels a little kludgy to have to create two SolutionBuilders. Might there be a way in the future to build all projects with a filter, or simply to build all dependencies of a project? Thanks for all your work on this! |
Thanks for continuing to look into this @andrewbranch. Given that this is a closed issue and I'm vaguely aware that not everyone tracks closed issues, I thought I'd open up a new issue specifically related to ts-loader project references build support. Aid visibility and all that. I've copied your comment above there. ANYONE READING THIS INTERESTED IN LEARNING ABOUT ADDING BUILD SUPPORT TO |
We have a monorepo when one of the packages is compiled with webpack, but references other package from the monorepo. We are using project references (new in TS 3) to compile only the packages, that are needed (the changed one and other that have reference to it). Is there a way to make it work with webpack? Currently we are compiling it with tsc and then bundling compiled js with webpack, but it is not an ideal solution, since many features doesn't work so well (e.g. source maps and custom loaders such as sass-loader) and it is quite slow.
The text was updated successfully, but these errors were encountered: