supply resolveTypeReferenceDirective to ts-loader

[johnnyreilly]

Now this is ballsy - I haven't tested this yet. But I'm guessing this is what we're going to need. Happy to report back when I've actually had chance to test this 😄

[arcanis]

Looks good to me! I guess the easiest to test is probably to use a git url in the package.json, right? I can also publish a release in rc state if you'd like.

[johnnyreilly]

Don't worry about publishing an RC. I was just going to test locally using the file protocol: https://docs.npmjs.com/files/package.json#local-paths

But now you mention it, git is a fine alternative. 😁

[johnnyreilly]

I haven't forgotten about this - I just want to get a response from the TypeScript team on this question first: TypeStrong/ts-loader#934

[johnnyreilly]

I think we're safe to merge now that @andrewbranch has kindly fixed up ts-loader 😄

[johnnyreilly]

Hey @arcanis,

I wanted to pick your brains. I've been working away on putting together some examples of how to use ts-loader with yarn PnP. I've got 2 examples in place:

yarn PnP with ts-loader

https://github.com/TypeStrong/ts-loader/tree/yarn-pnp-broken-example/examples/yarn

This works just fine as far as I can tell.

yarn PnP with ts-loader and fork-ts-checker-webpack-plugin

https://github.com/TypeStrong/ts-loader/tree/yarn-pnp-broken-example/examples/yarn-fork-ts-checker-webpack-plugin

This second example errors out with errors that suggest that fork-ts-checker-webpack-plugin is failing to resolve imports. So yarn build leaves you with a bunch of these:

ERROR in C:/source/ts-loader/examples/yarn-fork-ts-checker-webpack-plugin/src/app.tsx(1,24):
TS2307: Cannot find module 'react'.

It should be reproducible by doing the following:

git clone https://github.com/TypeStrong/ts-loader.git
cd ts-loader
git checkout yarn-pnp-broken-example
cd examples/yarn-fork-ts-checker-webpack-plugin
yarn install
yarn build

The code here is heavily inspired your PR to add PnP support to create-react-app with the fork-ts-checker-webpack-plugin: https://github.com/facebook/create-react-app/pull/6856/files

Do you have any idea why this might not be working?

side question

Do you think it would it make sense for pnpTs.js to live in pnp-webpack-plugin? Having written the fork-ts-checker-webpack-plugin config file it did occur to me that maybe it might be a better developer experience if this available in the pnp-webpack-plugin package itself.

What do you think?

const {resolveModuleName} = require('ts-pnp');

exports.resolveModuleName = (typescript, moduleName, containingFile, compilerOptions, resolutionHost) => {
  return resolveModuleName(moduleName, containingFile, compilerOptions, resolutionHost, typescript.resolveModuleName);
};

exports.resolveTypeReferenceDirective = (typescript, moduleName, containingFile, compilerOptions, resolutionHost) => {
  return resolveModuleName(moduleName, containingFile, compilerOptions, resolutionHost, typescript.resolveTypeReferenceDirective);
};

[arcanis]

Do you have any idea why this might not be working?

You need to bump fork-ts-checker-webpack-plugin to ^1.3.3 - my PR wasn't merged back in 1.0.0 😃

Do you think it would it make sense for pnpTs.js to live in pnp-webpack-plugin?

Maybe, yep. My concern was mostly that it's a quite fork-ts-checker-webpack-plugin-specific plugin, but at the same time if the DevX is improved with only a few lines it's probably worth it 👍

[johnnyreilly]

You need to bump fork-ts-checker-webpack-plugin to ^1.3.3 - my PR wasn't merged back in 1.0.0 😃

[Face hits desk]

Thanks - you're my idiocy linter 😄

Maybe, yep. My concern was mostly that it's a quite fork-ts-checker-webpack-plugin-specific plugin, but at the same time if the DevX is improved with only a few lines it's probably worth it 👍

Cool - should I add it to this PR or raise a different one? (I don't mind either way)

[arcanis]

Let's add it to this one! I guess we can use a similar API? Something like:

module.exports.forkTsCheckerOptions = (options = {}) => pnp ? Object.assign({}, options, {
  // ...
});

[johnnyreilly]

Oh I hadn't thought about it like that! Yeah I guess that makes sense 😁

Let me have a play and report back - might not get to do this immediately; about to go on hols

[arcanis]

No worry! Happy holidays 🍹🌴

[johnnyreilly]

Got this working using my original approach. (See linked PR)

I've just noticed you've implemented the suggested options - good work!

I'll try out the new API and report back 🤗

[johnnyreilly]

I've swapped to the new API. Take a look at this commit; mostly I just deleted code: TypeStrong/ts-loader@e59e4fa

😄

I think this means that dev-wise we're good to merge. We should probably update the README.md docs to document the new API for fork-ts-checker-webpack-plugin too first though. What do you think?

[johnnyreilly]

So if there are yarn 2 t-shirts are there also pnp-webpack-plugin t-shirts too? 😉

BTW I've started work on a blog post about using ts-loader / fork-ts-checker-webpack-plugin with PnP. I want to publish it after we get this merged / released. I was going put reference in to the GitHub issues that, if resolved, will make yarn PnP more of a "default" node experience. Are these the ones to mention would you say?

You can follow more on built in webpack support here:

webpack/enhanced-resolve#162

And on built in TypeScript support here:

microsoft/TypeScript#18896

[arcanis]

Awesome! I added a note in the README about fork-ts-checker-webpack-plugin, let me know what you think, we can easily tweak it. In the meantime I've released this PR as 1.5.0, if you want to start using it 😃

I was going put reference in to the GitHub issues that, if resolved, will make yarn PnP more of a "default" node experience. Are these the ones to mention would you say?

Yep! Another one (and I'm afraid it's more a repo than one particular issue) would be the nodejs/module repository, which debates amongst other things how to properly integrate loaders with Node. It would be nice because:

• We'd stop having to patch require
• We probably wouldn't have to use yarn node if Node itself was able to find the loader somehow (such as if it was listed in the package.json metadata)

So if there are yarn 2 t-shirts are there also pnp-webpack-plugin t-shirts too? 😉

We don't, but given how helpful you were I'd be happy to send you a Yarn tshirt 😊 Feel free to send me your contact info / tshirt size by mail and I'll see what I can do!

[johnnyreilly]

We don't, but given how helpful you were I'd be happy to send you a Yarn tshirt 😊 Feel free to send me your contact info / tshirt size by mail and I'll see what I can do!

Awesome 😘

Awesome! I added a note in the README about fork-ts-checker-webpack-plugin, let me know what you think, we can easily tweak it. In the meantime I've released this PR as 1.5.0, if you want to start using it 😃

Great work! That gives me an excuse to finish my blog post 😊

If it's cool with you I'll quote the following in some way in the blog post:

Yep! Another one (and I'm afraid it's more a repo than one particular issue) would be the nodejs/module repository, which debates amongst other things how to properly integrate loaders with Node. It would be nice because:

• We'd stop having to patch require
• We probably wouldn't have to use yarn node if Node itself was able to find the loader somehow (such as if it was listed in the package.json metadata)

Happy to take updates on the post when it's done BTW. Most of my posts have patch releases 😀

[johnnyreilly]

Hey @arcanis,

I've written usage of this up as this blog post:

https://blog.johnnyreilly.com/2019/06/typescript-webpack-you-down-with-pnp.html

Are there any tweaks you'd suggest? I haven't publicised it yet...

[arcanis]

Nice post! Some more info:

• I'd mention that people using PnP should probably try out the v2 (https://github.com/yarnpkg/berry), as it's where most of our work happen, including zip loading - two birds one stone!

• We have worked on a tool called PnPify that adds support for PnP to TypeScript (in particular tsc). You can find more information here: https://yarnpkg.github.io/berry/advanced/pnpify. For tsc it would be:

  $> yarn pnpify tsc [...]
  

  The gist is that it simulates the existence of node_modules by leveraging the data from the PnP file. As such it's not a perfect fix (pnp-webpack-plugin is a better integration), but it's a very useful tool to have to unblock yourself when using a project that doesn't support it. It might be worth mentioning!

• Further to that, VS Code (which is powered by TypeScript remember) has no support for Yarn PnP yet and so will present resolution errors to you.

  PnPify actually allows us to use TypeScript in VSCode with PnP! Its documentation is here:
  https://yarnpkg.github.io/berry/advanced/pnpify#vscode-support

• First things first, add this property to your package.json:

  You might want to mention that this tag will be optional starting from the v2, where projects will switch to PnP by default.

• Thanks to Maël for his tireless work on Yarn. To my mind Maël is certainly a candidate for the hardest worker in open source. I've been shamelessly borrowing his excellent docs for this post - thanks for writing so excellently Maël!

  Thanks ... 😊

[johnnyreilly]

Awesome points - thanks for adding! I'll update the post with these shortly Updated! Lemme know if I got anything wrong 😄