Add export specification

[tobiasdiez]

trafficstars

Define Package entry points using the exports field in package.json. There might be additional paths that you want to expose and I'm not sure if the utilities should be exposed by this package.json file or by its own one.

Fixes https://github.com/apollographql/apollo-client/issues/9048 and probably https://github.com/apollographql/apollo-client/issues/9938, https://github.com/apollographql/apollo-client/issues/9156, https://github.com/apollographql/apollo-client/issues/8218, https://github.com/apollographql/apollo-feature-requests/issues/287

Checklist:

• [ ] If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
• [ ] Make sure all of the significant new logic is covered by tests

[apollo-cla]

@tobiasdiez: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

[rosskevin]

Here is a reproduction of apollo failing as a pure ESM library import due to what I believe is the lack of proper exports (I could be wrong). Note that switch typescript to "moduleResolution": "node" instead of "moduleResolution": "nodenext" allows this to succeed, which supports the argument that the problem is exports based.

https://github.com/rosskevin/ts-esm-workspaces/tree/apollo-exports

[Mordred]

Here is my not working use case https://github.com/Mordred/apollo-client-esm

I created new project and added "type": "module" to package.json. NodeJS throws a SyntaxError that @apollo/client is CommonJS module.

It would be nice to have exports defined - but do not forget to add ./link/schema, ./link/ws and more which are not currently in this PR but are in the docs (e.g. https://www.apollographql.com/docs/react/api/link/apollo-link-schema/)

[tobiasdiez]

@Mordred Thanks for the suggestion. Could you provide the other exports as PR review suggestions, so I can easily commit them to this PR in your name?

@jpvajda @MrDoomBringer I would very much appreciate if you could find the time to review this PR.

[jpvajda]

👋 @tobiasdiez I noticed a weird Circle CI error on this PR, so I'm curious what might be occurring here. Jyst adding a few people from the team to check this out for you. @bignimbus @hwillson

Could not find a usable config.yml, you may have revoked the CircleCI OAuth app.Please sign out of CircleCI and log back in with your VCS before triggering a new pipeline.

[bignimbus]

Thanks for the tag @jpvajda and thanks for your contribution @tobiasdiez! I haven't been able to narrow down the issue with CircleCI - @tobiasdiez what do you see when you click the "Details" link next to "CircleCI Pipeline?"

I was able to get a build started by pushing a branch with these changes. See https://app.circleci.com/pipelines/github/apollographql/apollo-client/16912/workflows/ae50495f-993e-4359-94f8-b3b37dcd2502/jobs/93836 - it looks like there are some failing tests regarding modules:

     Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './apollo-client.cjs' is not defined by "exports" in /home/circleci/project/scripts/memory/node_modules/@apollo/client/package.json
     Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './react' is not defined by "exports" in /home/circleci/project/scripts/memory/node_modules/@apollo/client/package.json

Given @Mordred's comment, I think we need to take a closer look at our test coverage to ensure that all entry points are covered.

[tobiasdiez]

@bignimbus Thanks for investigating. I get a similar message on CircleCI:

Could not find a usable config.yml, you may have revoked the CircleCI OAuth app.Please sign out of CircleCI and log back in with your VCS before triggering a new pipeline.

It's also not working if I try to create a new PR: https://github.com/apollographql/apollo-client/pull/10086. So maybe its best if you (or someone else) creates a new PR with the changes.

[bignimbus]

Thanks for trying again @tobiasdiez 🙏🏻 I would definitely like to figure out what might be going on. Regardless of whether it's a broader issue with our CircleCI config or something happening in your specific account, I'd like to make sure you're fully able to contribute to our community. Would it be ok for me to reach out directly via your Twitter DMs next week and try a couple of debug steps? Completely fine if you'd prefer not to, whatever works for you!

[tobiasdiez]

I've now created a circleci account (via github oauth) and it seems like this did the job. Strange though...

[MrDoomBringer]

Hey @tobiasdiez ! Thanks for the ping on this.

The question of entrypoint coverage in this thread looks to me to be a bit of a challenge. To truly prevent a breaking change, we'd need to cover (just about) every export statement in @apollo/client/..., rather than only the module entrypoints defined in config/entrypoints.js and otherwise mentioned in the docs.

That would look something like this exports definition ...

"exports": {
    "./testing": {
      "import":"./testing/index.js",
      "require":"./testing/testing.cjs"
    },
    "./testing/core": {
      "import":"./testing/core/index.js",
      "require":"./testing/core/core.cjs"
    },
      "./testing/*": "./testing/*.js",
      "./testing/*.js": "./testing/*.js"
  }

... to support the public entrypoints we currently have in @apollo/client/testing, for example:

// foo.js
// > node --experimental-specifier-resolution=node foo.js
import * as a from '@apollo/client/testing/core/mocking/mockClient'
import * as b from '@apollo/client/testing/core/mocking/mockClient.js'
import * as c from '@apollo/client/testing/core/itAsync'
import * as d from '@apollo/client/testing/core/itAsync.js'
import * as e from '@apollo/client/testing/core'
import * as f from '@apollo/client/testing/react/MockedProvider'
import * as g from '@apollo/client/testing/react/MockedProvider.js'
import * as h from '@apollo/client/testing'

Even taking advantage of node's subpath export patterns I worry how maintainable our package.json will be after covering every public entrypoint we have right now.

Also, doing all this might be avoiding what the team at nodejs.org is trying to get us to do:

allowing module authors to clearly define the public interface for their package.

It might be that the correct move here includes targeting a later release (Maybe 4.0?), then some planning on what we want our public interface to be and reorganizing our codebase to make that interface more easily accessible.

I'm not yet totally certain on my answer here, please let me know if I missed something.

All the best, Emmanuel, Intern :-)

[tobiasdiez]

I guess it depends on what you define as the public interface. For example, direct imports of js files like '@apollo/client/testing/core/mocking/mockClient.js' I would consider as a private implementation detail whose public interface is '@apollo/client/testing/core/mocking/mockClient'.

But yes, in general, I agree it would be nice if apollos interface could be streamlined and many deep imports combined (e.g. @apollo/client/testing being the only entrypoint). But let's not do this as part of this PR.

[rosskevin]

@MrDoomBringer perhaps a breaking change is necessary. I agree that the apollo packaging is quite unconventional, but regardless, people using ESM are broken because apollo packages are not providing an updated picture of their package via the exports specification. I'm for a major bump if the dist can get cleaned up and the exports specified appropriately.

For what it is worth, here is my current yarn patch - I know that it is incomplete (indeed I already noted the lack of cjs file in the last entry)...but it is getting me by (so far).

diff --git a/package.json b/package.json
index 3a2bf8a3ead2ca597564f8fbfafa8af78c385a36..d4aee2f69d4e77e8bcf836265ce8bc2c2b10958a 100644
--- a/package.json
+++ b/package.json
@@ -16,6 +16,37 @@
   "main": "./main.cjs",
   "module": "./index.js",
   "types": "./index.d.ts",
+  "exports": {
+    ".": {
+      "types": "./index.d.ts",
+      "import": "./index.js",
+      "require": "./main.cjs"
+    },
+    "./core": {
+      "types": "./core/index.d.ts",
+      "import": "./core/index.js",
+      "require": "./core/core.cjs"
+    },
+    "./link/core": {
+      "types": "./link/core/index.d.ts",
+      "import": "./link/core/index.js",
+      "require": "./link/core/error.cjs"
+    },
+    "./link/error": {
+      "types": "./link/error/index.d.ts",
+      "import": "./link/error/index.js",
+      "require": "./link/error/error.cjs"
+    },
+    "./testing": {
+      "types": "./testing/index.d.ts",
+      "import": "./testing/index.js",
+      "require": "./testing/testing.cjs"
+    },
+    "./utilities/policies/pagination": {
+      "types": "./utilities/policies/pagination.d.ts",
+      "import": "./utilities/policies/pagination.js"
+    }
+  },
   "sideEffects": false,
   "react-native": {
     "./dist/cache/inmemory/fixPolyfills.js": "./cache/inmemory/fixPolyfills.native.js",

[revmischa]

I think it makes total sense to simplify the public interface and release as 4.0.0 All of the @apollo/client code is getting bundled in both the client and server-side code of my application which makes web pages slower to load and lambda graphql resolver functions slower in cold starts. If this can help, it will be a great improvement for my users. Ref: https://github.com/apollographql/apollo-feature-requests/issues/287#issuecomment-1264435005

[tobiasdiez]

How should we proceed here? Is there something I can do to move this forward?

[phryneas]

@revmischa This is something we cannot do without a 4.0, and we will not do a 4.0 just for this. Reviewing the whole bundling setup is something we will be doing for the next major, especially with ESM in mind.

Unfortunately, there is still no generally-agreed way of serving ESM without breaking some build or bundling tools. For example, see over in the Redux repo, where we are trying to do this at this moment. Every change published in an alpha generates a bug report that something is broken.

At this point, it's probably best that you use something like the patch suggested in https://github.com/apollographql/apollo-client/pull/9697#issuecomment-1293815758 until we get ready for the next major (and someone hopefully figures this problem space out before that).

[JasonKleban]

Related #9976

[phryneas]

I just mentioned this over in #10620 - at the point where we get to this change (definitely in the next major release!), we will likely have to adopt tooling that will autogenerate these exports fields.

As I'm doing some housekeeping, I'm going to close this PR for now - that definitely doesn't mean that we won't do this though! Really appreciate the PR and bringing this up 👍