query icon indicating copy to clipboard operation
query copied to clipboard

fix/umd-build

Open JohnDaly opened this issue 1 year ago • 5 comments

UMD builds are configured to always generate output files with the format: ${packageDir}/build/umd/index.production.js and ${packageDir}/build/umd/index.development.js.

This is a problem, because there are two sets of build configs for the dev tools package: react-query-devtools and react-query-devtools-noop. Since they are both generating output files with the same name, this causes react-query-devtools-noop to always override the output of react-query-devtools.

To fix this, I updated the umdDev and umdProd functions to use the outputFile variable to determine the name of the output file. Then I provided different outputFile values for react-query-devtools and react-query-devtools-noop

JohnDaly avatar Jul 26 '22 06:07 JohnDaly

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit d85684bb3b6e9d6b78388c46a7936130ad019690:

Sandbox Source
@tanstack/query-example-react-basic Configuration
@tanstack/query-example-react-basic-typescript Configuration

codesandbox-ci[bot] avatar Jul 26 '22 06:07 codesandbox-ci[bot]

This change should allow for a new react-query-devtools/build/umd/index.development.js to be generated, which can be used to fix https://github.com/TanStack/query/issues/3916

JohnDaly avatar Jul 26 '22 06:07 JohnDaly

Codecov Report

Merging #3924 (d85684b) into beta (e0aad73) will decrease coverage by 0.33%. The diff coverage is n/a.

@@            Coverage Diff             @@
##             beta    #3924      +/-   ##
==========================================
- Coverage   97.11%   96.77%   -0.34%     
==========================================
  Files          50       57       +7     
  Lines        2391     2666     +275     
  Branches      706      782      +76     
==========================================
+ Hits         2322     2580     +258     
- Misses         67       84      +17     
  Partials        2        2              
Impacted Files Coverage Δ
.../persistQueryClient/PersistQueryClientProvider.tsx
src/core/hydration.ts
src/tests/utils.ts
src/core/removable.ts
src/core/focusManager.ts
src/devtools/styledComponents.ts
src/core/query.ts
src/core/queriesObserver.ts
src/core/infiniteQueryObserver.ts
src/reactjs/useSyncExternalStore.ts
... and 97 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov-commenter avatar Jul 26 '22 16:07 codecov-commenter

@JohnDaly we want to release all build related things in a pre-release version. There is also this PR that re-adds proper ESM support:

  • https://github.com/TanStack/query/pull/3892

both of them should go to the beta branch please, and there are likely conflicts. thanks 🙏

TkDodo avatar Jul 28 '22 08:07 TkDodo

This bug is preventing me from upgrading to v4 as I currently have to use an older version of react-scripts. When do you think this will be merged?

phil-saam avatar Aug 03 '22 19:08 phil-saam

@JohnDaly it seems that this PR increased the bundle-size of react-query by 5x 😅

https://bundlephobia.com/package/@tanstack/[email protected]

We can also see this in the bundlewatch logs:

packages/react-query/build/umd/index.production.js: 43.89KB (gzip)

size on main is:

packages/react-query/build/umd/index.production.js: 11.37KB (gzip)

any idea what that could be about?

TkDodo avatar Aug 12 '22 07:08 TkDodo

@JohnDaly it seems that this PR increased the bundle-size of react-query by 5x 😅

https://bundlephobia.com/package/@tanstack/[email protected]

We can also see this in the bundlewatch logs:

packages/react-query/build/umd/index.production.js: 43.89KB (gzip)

size on main is:

packages/react-query/build/umd/index.production.js: 11.37KB (gzip)

any idea what that could be about?

I ran the build on the beta branch, and noticed that react-dom was being included:

Beta build

Screen Shot 2022-08-12 at 9 02 12 AM

Main build

Screen Shot 2022-08-12 at 9 02 41 AM

I applied this changes from this PR onto the code in the main branch, and it created a UMD production build that did not include react-dom:

packages/react-query/build/umd/index.production.js... 
index.production.js ⏤  11.6 kB

There are a few things that I noticed that are different between the beta branch and the main branch, that might be contributing to this:

1. The Rollup configs are different

Specifically the main branch doesn't include some of the globals that are specified in the beta branch.

Example '@tanstack/query-core': 'QueryCore' is not present in main, but is in beta:

    ...buildConfigs({
      name: 'react-query',
      packageDir: 'packages/react-query',
      jsName: 'ReactQuery',
      outputFile: 'index',
      entryFile: 'src/index.ts',
      globals: {
        react: 'React',
-       '@tanstack/query-core': 'QueryCore',
      },
    }),

2. The package.json for packages are different

There are changes to the fields that bundlers use to determine how to resolve packages. We can see that the entrypoints (e.g. module, main, exports) are quite different between branches and that the sideEffects field is as well:

  • Beta: https://github.com/TanStack/query/blob/4a4543c39ae500b537000c0cab807327d3ce5ff3/packages/react-query/package.json
  • Main: https://github.com/TanStack/query/blob/b3201e991d8737d2a4ddbb99324f3282156a19ef/packages/react-query/package.json

Example: Partial diff for the package.json file from "@tanstack/react-query"

"name": "@tanstack/react-query",
- "module": "build/lib/index.mjs",
+ "module": "build/esm/index.js",
- "main": "build/lib/index.js",
+ "main": "build/cjs/react-query/src/index.js",
  "browser": "build/umd/index.production.js",
- "types": "build/lib/index.d.ts",
+ "types": "build/types/packages/react-query/src/index.d.ts",
- "exports": {
-   ".": {
-     "types": "./build/lib/index.d.ts",
-     "import": "./build/lib/index.mjs",
-     "default": "./build/lib/index.js"
-   },
-   "./package.json": "./package.json"
- },
  "sideEffects": [
-   "./src/setBatchUpdatesFn.ts"
+   "lib/index.js",
+   "lib/index.mjs",
+   "lib/reactjs/index.js",
+   "lib/reactjs/index.mjs",
+   "lib/reactjs/setBatchUpdatesFn.js",
+   "lib/reactjs/setBatchUpdatesFn.mjs"
  ],

JohnDaly avatar Aug 12 '22 15:08 JohnDaly

@JohnDaly Yeah basically it's due to missing global declaration of react-dom. I will make the fix for it. Tested already that it fixes the issue.

DamianOsipiuk avatar Aug 12 '22 16:08 DamianOsipiuk