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 incorrect usage of package.json types/main in package subdirectories #60696

Conversation

andrewbranch
Copy link
Member

Fixes a bug I noticed randomly

The content of moduleResolution_packageJson_yesAtPackageRoot.ts, for easier reviewing:

// @noImplicitReferences: true
// @traceResolution: true

// @Filename: /node_modules/foo/bar/index.js
not read

// @Filename: /node_modules/foo/package.json
{ "name": "foo", "version": "1.2.3", "types": "types.d.ts" }

// @Filename: /node_modules/foo/types.d.ts
export const x = 0;

// @Filename: /a.ts
import { x } from "foo/bar";

You can see the resolution of "foo/bar" incorrectly looking for /node_modules/foo/bar/types.d.ts due to package.json content in /node_modules/foo. (It doesn’t change the final resolution in these tests because /node_modules/foo/bar/types.d.ts doesn’t exist.) Presumably this problem only escaped detection for ~10 years because whatever value was in package.json "types" usually is either "index.d.ts", which would get looked up in subdirectories anyway, or is a path that doesn’t exist when resolved relative to the subdirectory path.

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Dec 5, 2024
@andrewbranch
Copy link
Member Author

@typescript-bot test top400

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 5, 2024

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
test top400 ✅ Started

@andrewbranch
Copy link
Member Author

@typescript-bot test top400

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 6, 2024

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
test top400 ✅ Started ✅ Results

@typescript-bot
Copy link
Collaborator

@andrewbranch Here are the results of running the top 400 repos with tsc comparing main and refs/pull/60696/merge:

Everything looks good!

@andrewbranch andrewbranch merged commit 421f5c5 into microsoft:main Dec 6, 2024
32 checks passed
@andrewbranch andrewbranch deleted the bug/module-resolution-package-json-field-subdir branch December 6, 2024 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants