Skip to content

Commit

Permalink
Remove ESM wrapper (#312)
Browse files Browse the repository at this point in the history
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:

```json
{
  "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.
  • Loading branch information
paul-sachs committed Dec 20, 2023
1 parent 0c7e80d commit 6d02d78
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 110 deletions.
9 changes: 2 additions & 7 deletions packages/connect-query/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@
},
"scripts": {
"clean": "rm -rf ./dist/*",
"build": "npm run build:cjs && npm run build:esm && npm run build:proxy",
"build": "npm run build:cjs && npm run build:esm",
"build:cjs": "tsc --project tsconfig.build.json --module commonjs --outDir ./dist/cjs --declaration --declarationDir ./dist/cjs && echo >./dist/cjs/package.json '{\"type\":\"commonjs\"}'",
"build:esm": "tsc --project tsconfig.build.json --module ES2015 --verbatimModuleSyntax --outDir ./dist/esm --declaration --declarationDir ./dist/esm",
"build:proxy": "node ../../scripts/gen-esm-proxy.mjs .",
"generate": "buf generate --path eliza.proto",
"test": "jest",
"format": "prettier . --write --ignore-path ./.eslintignore && eslint . --fix && license-header",
Expand All @@ -24,12 +23,8 @@
"main": "./dist/cjs/index.js",
"exports": {
".": {
"node": {
"import": "./dist/proxy/index.js",
"require": "./dist/cjs/index.js"
},
"module": "./dist/esm/index.js",
"import": "./dist/proxy/index.js",
"import": "./dist/esm/index.js",
"require": "./dist/cjs/index.js"
}
},
Expand Down
103 changes: 0 additions & 103 deletions scripts/gen-esm-proxy.mjs

This file was deleted.

0 comments on commit 6d02d78

Please sign in to comment.