Build generates invalid package.json files, thereby breaking tooling (eg normalize-package-data, CycloneDX)

[AugustusKling]

Issue Description

Code in

apollo-client/config/prepareDist.js

Line 64 in b214dd1

// Create individual bundle package.json files, storing them in their

generates package.json files for subfolders that are not real NPM packages. It uses multiple slashes as part of the package names which makes them invalid. If I understand the comment #6656 (review) correctly this is done to allow for partial imports.

We stumbled over this when trying to use CycloneDX create an SBOM for a project. The presence of the package.json files above broke the run of CycloneDX thereby making it imposisble to use it as soon as @apollo/client is a dependency of the project,

I guess the aim of allowing partial imports of @apollo/client could be archived in a safer manner by either:

• Use imports of mts or mjs files and get rid of the package.json files that supposedly only exist to transport the type=module setting to NodeJS.
• Split up @apollo/client in various smaller packages whilst having transitive dependencies in @apollo/client for those users who want the complete Apollo client.

Please correct me if I misunderstood the reasons for having the package.json files generated for your subfolders.

References:

• Ticket in CycloneDX: [BUG]: package normalization might cause crashes CycloneDX/cyclonedx-webpack-plugin#745
• Ticket in normalize-package-data: [BUG] Invalid name: "@apollo/client/core" npm/normalize-package-data#175

Link to Reproduction

https://github.com/sresch4b/cyclonedx-issue

Reproduction Steps

Get repo from https://github.com/sresch4b/cyclonedx-issue

yarn install

yarn build

[benjamn]

Do you have the option of using the ESM module tree (e.g. @apollo/client/index.js and other .js files)? Thanks to the normalization of imported identifiers performed by this build script, all internal ESM imports are fully normalized (with file extensions), so the contents of the internal/nested package.json files should have no influence on ESM module resolution.

If you're only able to use the CommonJS entry point (e.g. @apollo/client/main.cjs), I would imagine your bundling tools already have to achieve a certain level of compatibility with Node.js, and nested package.json files are definitely allowed in the Node.js execution model, even if they are not published as standalone top-level npm packages. We have published loose packages before, and versioning was a nightmare.

[phryneas]

Hi Augustus,
unfortunately, this is nothing we could eliminate in the short or medium term. I'll give some context.

This practice of having sub-folders with package.json files to enable imports like module/subfolder has been coined "proxy directory". The earliest mention where I personally encountered this practice was in this discussion on the tsdx repo.

Before the exports field in package.json, this was essentially the de-facto way of enabling more than one export to work best with as many bundlers (and TypeScript) as possible. It's not enough to have an index.ts there since different bundlers might want to pick up different bundle formats. So we need the ability to specify a main and module entry point, other packages use this for even more bundle format.
For example, @reduxjs/toolkit/query/react has main, module, and unpkg entry points defined in there. Another example of a popular library doing this was mentioned in the thread I linked: @emotion/styled/base (main, module, umd:main, browser entry points)

While we generate those proxy directories with package.json files by hand, tooling like Preconstruct even generates those for you as a bundling step - so you can expect that this is a practice shared by hundreds or even thousands of libraries depending on that tooling.

As it currently stands, there is no way around using these files in many libraries. exports might be a solution in the long run, but adoption has been slow, and removing those files would have a significant impact on the ecosystem and our consumers - many of which are using bundlers that we might not even have heard about, but which depend on this behavior.

I'm sorry to tell you this, but this seems like either you'll have to play that ball back to normalize-package-data, or CycloneDX will have to just ignore (or not validate) all package.json files that are not in the root folder of a package.

[AugustusKling]

Hi Ben and Lenz,

I really appreciate your feedback. Thank you.

The issue is not about importing the files so changing the way we import them does not matter. It's the presence of package.json files which contain name-properties that are invalid from the perspective of an NPM repo which breaks the build. Generating the name-properties from the folder structure works fine for importing in NodeJS and other UI builds but conflicts with the expectations tools like normalize-package-data have, see references in initial post.

The answers you provided here helped to understand better why Apollo chose to do it this way. Always good to understand the reasons behind doing things a certain way.

I think we can conclude that the build in @apollo/client needs to stay the way it is as changing it would be breaking for a lot of users. It also means that CycloneDX needs to adapt and learn to ignore such package.json files in subfolders/proxy directories.

Fortunately, @jkowalleck has already prepared CycloneDX/cyclonedx-webpack-plugin#754 in an attempt to make CycloneDX more robust in this respect. If everything goes well this should allow CycloneDX to produce SBOMs even if @apollo/client is a depedency of a project.

To give some context: the goal is then to use this SBOM in mainly two ways:

• Verify no transitive dependency introduces unwanted licences.
• Have the SBOM as a build artifact to be regularly scanned for security vulnerabilities. The audit-command of the package manager is not good enough here since the NPM dependencies are only one part of the deployed system.

Getting this working correctly is actually surprisingly difficult given the various ways licences are referred to and other system parts like Docker containers and backend code that also needs to be considered.

Thanks again for your input. I'll go ahead an close this ticket as the work on this needs to continue elsewhere.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
For general questions, we recommend using StackOverflow or our discord server.