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

Remove ESM wrapper #312

Merged
merged 5 commits into from
Dec 20, 2023
Merged

Remove ESM wrapper #312

merged 5 commits into from
Dec 20, 2023

Conversation

paul-sachs
Copy link
Collaborator

@paul-sachs paul-sachs commented Dec 19, 2023

Fixes #305... again again.

The esm wrapper (as described in bufbuild/protobuf-es#509) creates problems when using a peer dependency that defines it's exports without an import proxy (in our case, react-query).

Current situation

Our exports look like:

{
  "exports": {
    ".": {
      "node": {
        "import": "./dist/proxy/index.js",
        "require": "./dist/cjs/index.js"
      },
      "module": "./dist/esm/index.js",
      "import": "./dist/proxy/index.js",
      "require": "./dist/cjs/index.js"
    }
  }
}

The above meant the following:

  • When the application imported QueryClientProvider from @tanstack/react-query, it resolved to the esm version of the context
  • When the library imported useQuery from @tanstack/react-query, it resolved instead to the cjs version of the package (since NextJS SSR is considered a "node" environment)

This mismatch between the esm and cjs versions causes a problem detecting context

Alternatives considered

We could expose our own hooks/components with a one-to-one mapping from @tanstack/react-query and require our own QueryClientProvider to be initialized. This would bypass these kinds of issues, but introduce more setup and potential for confusion. It would mean that using react-query for anything non-connect related would require an additional query client provider which MAY actually be the same client depending on where (node/browser) it's running.

Reason we chose this solution

By not using an ESM wrapper, we avoid these possible import mismatches. It does introduce the possibility of dual package hazard with class instances (where instanceof is used, like with ConnectError) but we don't use any of that in connect-query so we can be relatively safe in that regard.

@paul-sachs paul-sachs changed the title Match exports provided by react-query Match package.json exports provided by react-query Dec 19, 2023
packages/connect-query/package.json Outdated Show resolved Hide resolved
packages/connect-query/package.json Outdated Show resolved Hide resolved
@paul-sachs paul-sachs changed the title Match package.json exports provided by react-query Remove ESM wrapper Dec 20, 2023
@paul-sachs paul-sachs merged commit 6d02d78 into main Dec 20, 2023
8 checks passed
@paul-sachs paul-sachs deleted the psachs/module-consistency branch December 20, 2023 15:46
@paul-sachs paul-sachs mentioned this pull request Dec 20, 2023
paul-sachs added a commit that referenced this pull request Dec 20, 2023
## What's Changed
* Remove ESM wrapper by @paul-sachs in
#312


**Full Changelog**:
v1.1.1...v1.1.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Getting "No QueryClient set, use QueryClientProvider to set one" error in version 1.0.0
2 participants