Skip to content
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

Fix auto-import when paths points to project reference redirect #51492

Merged
merged 2 commits into from
Nov 15, 2022

Conversation

andrewbranch
Copy link
Member

Fixes #51269

@andrewbranch
Copy link
Member Author

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Nov 11, 2022

Heya @andrewbranch, I've started to run the tarball bundle task on this PR at 4f5d243. You can monitor the build here.

@microsoft microsoft deleted a comment from typescript-bot Nov 11, 2022
@microsoft microsoft deleted a comment from typescript-bot Nov 11, 2022
@andrewbranch andrewbranch marked this pull request as ready for review November 11, 2022 22:34
@typescript-bot
Copy link
Collaborator

Hey @andrewbranch, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/138465/artifacts?artifactName=tgz&fileId=B24E168A5F156C443B7715945540EF9C883F6507E423099809ECA2957D1C1B3702&fileName=/typescript-5.0.0-insiders.20221111.tgz"
    }
}

and then running npm install.

Comment on lines +24 to +26
//// "paths": {
//// "@common/*": ["../common/dist/src/*"]
//// }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Offtopic question: I was actually exploring using exactly this structure in one of the projects. However, I only want this because Remix doesn't allow me to alias paths - but it can infer aliases from the compilerOptions.paths.

I wonder what are the pros/cons of this structure. I guess that in this test case here it might be required to use non-relative paths as /common/ isn't actually symlinked as a monorepo package. However, when referenced projects are symlinked in node_modules, I expect this to be unnecessary and less performant. In the IDE this might be roughly the same thanks to "behind-the-scenes in-memory .d.ts generation process" but when running tsc -b this might not be able to actually leverage the generated .d.ts files as the paths map to /src/, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, in this example paths map to dist (the original repro didn’t set a rootDir so the output is in dist/src and I kept that quirk in the test). But IIRC whether you map to the inputs or outputs of a referenced project, TypeScript understands what you’re doing and tsc will find the .d.ts outputs and the language service will find the .ts inputs.

@andrewbranch andrewbranch merged commit 89ce16c into microsoft:main Nov 15, 2022
@andrewbranch andrewbranch deleted the bug/51269 branch November 15, 2022 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto import of monorepo package not using node_modules specifier
5 participants