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

[node-core-libary] Update PackageJsonLookup to only resolve to package.json files that contain a "name" field. #3559

Merged
merged 4 commits into from
Aug 18, 2022

Conversation

eemeli
Copy link
Contributor

@eemeli eemeli commented Jul 28, 2022

Summary

Fixes #2070

PackageJsonLookup is erroneously presuming that all package.json files include a "name" field, and will complain when attempting to load a package.json file that does not include that field.

In the npm docs on the file format, it's quite clear that the name+version requirements only apply to published packages:

If you don't plan to publish your package, the name and version fields are optional.

This means that when e.g. a third-party dependency follows the guidance given for the "type" field in the Node.js documentation, it's not possible to successfully run api-extractor.

Details

To fix this, each package.json file is loaded during lookup before returning it as valid, and in case the load fails with the The required field "name" was not found error, the lookup continues in the parent directory.

This should have no real-world effect on performance, as practically all such lookups are immediately followed by a load, which will then use the result cached during the lookup.

How it was tested

A test case is added to validate this behaviour.

@ghost
Copy link

ghost commented Jul 28, 2022

CLA assistant check
All CLA requirements met.

@iclanton iclanton changed the title [node-core-libary] Require at least a "name" field in package.json files [node-core-libary] Update PackageJsonLookup to only resolve to package.json files that contain a "name" field. Aug 18, 2022
@iclanton iclanton enabled auto-merge August 18, 2022 03:30
@iclanton iclanton merged commit 510809a into microsoft:main Aug 18, 2022
@eemeli eemeli deleted the fix-2070 branch August 18, 2022 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[api-extractor] Partial package.json files (used by bundlers for tree shaking) prevent .d.ts rollup
4 participants