Custom resolveModuleName

[arcanis]

Now that support has landed in ts-loader for a custom resolveModuleName option, would fork-ts-checker-webpack-plugin be interested in a similar option?

My own use case it to make this plugin work with PnP (which is natively supported by create-react-app, itself using fork-ts-checker-webpack-plugin to power its TypeScript part) 🙂

[johnnyreilly]

Hey @arcanis!

That's a super kind offer and it'd be greatly appreciated! However before you put effort in, I think there might be something that needs to be resolved first.

Right now, create-react-app are actually using a fork of fork-ts-checker-webpack-plugin (very meta 😄). @Timer is looking at submitting a PR that makes the changes that will be required to make this happen. Predominantly, I believe that these amount to dropping the peerDependencies. See comment. We're pretty motivated to make the changes necessary so that CRA doesn't have to depend upon a fork.

See the full discussion about making that PR here: Timer#1

By the sounds of it, this will break PnP. To quote @Timer:

@johnnyreilly hey! I mentioned this in Realytics#165 (comment), but dropping the peer dependency is a hard requirement for us.

If you're OK with dropping the peer dependency, I'm more than happy to refine this PR and send it upstream!

(yes I know this breaks Yarn PnP but I'm OK with this)

By the sounds of it, create-react-app is not compatible with PnP because CRA has a hard requirement to not have peerDependencies and PnP has the exact opposite requirement. 😄

I would love CRA and PnP to both work with the fork-ts-checker-webpack-plugin - but I'm not sure how that can work.

Do you have any insight? I'm genuinely not sure what to do here.

[Timer]

Most of this is going to be painful until yarnpkg/yarn#6487 is supported. I'd love to have more open discussion about this.

Don't let our change block this feature, though! I'm sure we can figure out a way to integrate both of these changes.

[johnnyreilly]

Thanks for responding @Timer!

@arcanis do you have an idea of how we can serve both PnP and CRA's needs? It would be great to support both. I've not been closely tracking the PnP development so please feel free to give guidance; you're obviously going to know most on this particular topic 😄

[arcanis]

Hey!

We're pretty motivated to make the changes necessary so that CRA doesn't have to depend upon a fork.

Totally makes sense 👍

By the sounds of it, create-react-app is not compatible with PnP because CRA has a hard requirement to not have peerDependencies and PnP has the exact opposite requirement.

In the case of create-react-app, yes. In the more general case of an application depending on fork-ts-checker-webpack-plugin it will work just fine (the Yarn implementation of PnP has a fallback mechanism that defers to the top-level dependencies when an undeclared dependency is required).

Regarding optional peer dependencies, we're still discussing the details (cf the rfc thread) but I plan to ship them starting from Yarn 1.13 (in a month or so). There's no telling how much time it will take for npm to catch up, though, assuming they even want to do it (I've not heard from them yet). Would that be a blocker, @Timer?

Do you have an idea of how we can serve both PnP and CRA's needs? It would be great to support both

I think the best would be to:

1. Land the resolveModuleName option in fork-ts-checker-webpack-plugin; this will allow non-cra applications to start using PnP, plus it's useful even outside of the PnP use case
2. In parallel I continue my work on the optional peer dependencies, and it eventually lands in the 1.13.
3. Once everyone feel confident it won't cause unnecessary pain for users, the peer dependencies can be added back to fork-ts-checker-webpack-plugin using the optional peer dependencies.

How does that sound?

[Timer]

@arcanis as long as npm has a plan to implement I think we'd be fine adding the peer dep back (assuming backwards compatibility).

[arcanis]

Yep, optional peer dependencies will be backward compatible (they'll just generate a warning on older package managers).

[johnnyreilly]

That sounds great @arcanis!

Just so we're all clear: from the sounds of your plan we're safe to continue with plans to drop peerDependencies for now? So @Timer is still good to submit a PR in order that CRA no longer needs to depend upon the alt fork?

[johnnyreilly]

Is that about right @arcanis? Sorry to chase - just wanted to make sure I understood your meaning.

[arcanis]

Missed your answer - yep, dropping the peer dependencies is fine by me (don't get me wrong: it has potential to break things even without PnP; but in practice it should work in most cases). I'll setup a PR to add them back as soon as we get optional peer dependencies (and then you'll be free to merge it whenever you feel ready), and in the meantime I'll setup another PR for resolveModuleName 😃

[johnnyreilly]

That's awesome - thanks so much!