Skip to content

Yarn PnP support - resolveTypeReferenceDirective - "I'm calling for a resolution..." #934

Closed
@johnnyreilly

Description

@johnnyreilly

It all started when @arcanis raised a PR which added Yarn PnP support to ts-loader. To quote the PR:

This PR adds a new resolveModuleName option that can be used to help TypeScript figure out the location of the files on the disk. It mimics the TypeScript API.

#862

Time passed and it was all good. Then the wonderful @arcanis submitted a very similar PR to the fork-ts-checker-webpack-plugin:

TypeStrong/fork-ts-checker-webpack-plugin#250

This PR was very similar to the ts-loader one. However, as I was reviewing it I noticed something. TypeStrong/fork-ts-checker-webpack-plugin#250 (comment)

To quote myself:

Thanks! I was just looking at the work you did on ts-loader: #862

You only added resolveModuleName there.

This adds 2 options: resolveModuleNameModule and resolveTypeReferenceDirectiveModule

Does ts-loader need a resolveTypeReferenceDirective to work with pnp as well? Or is it not necessary?

And @arcanis response:

I think that might be useful yep. When I initially made the PR for ts-loader I wasn't aware of what resolveTypeReferenceDirective was used for (/// <reference types="...">) - I just discovered it when implementing the feature for fork-ts-checker-webpack-plugin and only because create-react-app uses it to automatically define the *.svg typings.

So I decided to add the corresponding functionality to ts-loader; I gave it its own resolveTypeReferenceDirective:

#921

Now up until this point, when we created a ServicesHost in ts-loader we didn't actually supply resolveTypeReferenceDirective at all - in fact it was commented out in the codebase as you can (slightly embarrassingly) see here:

https://github.com/TypeStrong/ts-loader/pull/921/files#diff-70b0dbb6219cb6164e67ffb54a8f23c0L140

And it turned out that introducing it at all has had a breaking side effect for jest users:

#919 (comment)

Clearly this issue could be "fixed" by just dropping the resolveTypeReferenceDirectives from ts-loader. But if we do that, then I'm assuming we break Yarn PnP support too (only for type reference directives that is).

Where I am is here: I can no longer remember the reason I commented out supplying resolveTypeReferenceDirectives. Flat out, I just don't remember. And so we finally get to the question:

Is ts-loader using this API correctly? Should we be using differently? Or is it actually not to be used? I'm after guidance. Please educate me!

cc @andrewbranch

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions