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

[gregjoeval]

Please prefix the issue title with the project name i.e. [rush], [api-extractor] etc.

Is this a feature or a bug?

• [ ] Feature
• [x] Bug

Please describe the actual behavior. Valid partial package.json files of a dependency result in the following error message after executing api-extractor run

Error reading "F:\Projects\GitHub\package-library\packages\material-ui-adjunct\node_modules@material-ui\core\Grid\package.json": The required field "name" was not found

If the issue is a bug, how can we reproduce it? Please provide detailed steps and include a GitHub branch if applicable. Your issue will get resolved faster if you can make it easy to investigate. In a project that uses a package with partial package.json files (for example: Material-UI), prepare the project for api-extractor with .d.ts rollup, execute api-extractor run.

What is the expected behavior? Successfully execute api-extractor run with .d.ts rollup enabled while using packages with partial package.json files

If this is a bug, please provide the tool version, Node.js version, and OS.

• Tool: @microsoft/api-extractor
• Tool Version: ^7.9.2
• Node Version: v13.9.0
  • Is this a LTS version? no
  • Have you tested on a LTS version? no
• OS: Windows 10

[gregjoeval]

I can provide a more in depth example in one of my own repos if necessary

[vidartf]

Put in a different way: Certain third-party dependencies might put invalid package.json files at random locations in their source tree and that might work perfectly fine for them (e.g. https://github.com/mui-org/material-ui/issues/15715 to add context to the OP example). The question then becomes, is there a way for AE to gracefully handle such files, or is this a strict incompatibility?

[octogonz]

material-ui's package.json files violate at least two specifications for this file:

• CommonJS: name and version are required fields: http://wiki.commonjs.org/wiki/Packages/1.0
• NPM: name and version are required fields: https://docs.npmjs.com/files/package.json

The question then becomes, is there a way for AE to gracefully handle such files, or is this a strict incompatibility?

If we make a special accommodation for material-ui, what would it be?

In that thread, @eps1lon wrote:

These are not standalone packages. The package.json is just there to assist bundlers in finding the module builds. Is there a documentation for required name fields on npm? Otherwise this sounds like the api-extractor tool should just ignore those packages.

So the questions would be:

1. Will API Extractor produce correct output if we simply ignore these invalid files?
2. API Extractor's main role is to protect against mistakes for professionally shipped software, so generally it does not have a philosophy of silently ignoring errors. How should we determine whether a file is safe to ignore? (Missing name field?)

[gregjoeval]

Are there other packages that utilize a similar method of bundle splitting? If this is unique to material-ui maybe it would be better to work with them to figure out a way to add those properties to their partial package.json files

[eps1lon]

Are there other packages that utilize a similar method of bundle splitting? If this is unique to material-ui maybe it would be better to work with them to figure out a way to add those properties to their partial package.json files

Check out this twitter thread: https://twitter.com/mjackson/status/1295726938833657856

It lists a couple of packages that use this method which just leverages the node resolution algorithm.

CommonJS: name and version are required fields: http://wiki.commonjs.org/wiki/Packages/1.0

This defines the top-level package.json. It does not specify how nested package.json should look.

I think the same applies to npm though they don't explicitly mention top-level package.json

[octogonz]

This defines the top-level package.json. It does not specify how nested package.json should look.

How would we detect a "top-level" package.json? Node.js module resolution works by crawling directory trees. It doesn't really distinguish installed NPM packages versus local projects.

[vidartf]

Note that for NPM it says

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

[vidartf]

So the questions would be:

1. Will API Extractor produce correct output if we simply ignore these invalid files?

2. API Extractor's main role is to protect against mistakes for professionally shipped software, so generally it does not have a philosophy of silently ignoring errors.  How should we determine whether a file is safe to ignore?  (Missing `name` field?)

For the record, I do believe these are the questions that need answering.

Re pt 2: I agree with this philosophy, but make the following notes:

• For this case, it will also throw this exception if the package.json is in a dependency (i.e. for our example case not just for materialui, but for any project that depends on it directly or indirectly).
• The exception in question is uncaught. It should at least be caught and turned into a warning that is possible to ignore, instead of dumping a full trace to the output for older versions of Node (and for newer/future versions of Node, unhandled rejections in promises will cause the app to fall over, it warns). As such, we might not need to make the call about whether it is safe to ignore or not, but leave it to the user.

Re pt1: That depend on whether or not AE uses the name/version fields for anything in the code, or if it just checks out of caution. Normally NPM will prevent you from publishing if the package.json files does not include a name/version field, so the check might be redundant if AE doesn't actually use the fields for anything.

[eps1lon]

How would we detect a "top-level" package.json?

I don't know since I'm not using api-extractor nor involved with its implementation.

I'm just pointing out that this pattern is not violating any standard, is used beyond Material-UI and respected by webpack (and I believe rollup and possible every other bundler).

If you decide to keep throwing when encountering this pattern then it'd be nice if you could advise on an alternate implementation that does not break app bundles produced by webpack or rollup.

[sunnysingh]

Hi, any updates on this issue? We are currently unable to use API Extractor since it doesn't work with Material-UI. We could use something like npm.im/patch-package but it also seems to have issues with Rush, plus an official fix would be more ideal than a monkeypatch.

[jc-1234]

Any updates? This is preventing us from upgrading to PrimeReact v6.5 because they have package.json files in subfolders of their components that are missing name property.

[sebisteiner]

This is also a problem with maplibre-gl: https://unpkg.com/browse/[email protected]/dist/package.json

[matthewborgman]

Is there an intent to try to fix this? If not, this seems like the issue should be called out somewhere in the documentation.

Like @sunnysingh, my project is unable to use this tool, as much as we'd like, due to an "intermediary" TailwindCSS package that does not contain the name field.