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

fix: remove browser entry, fix umd size #4044

Merged
merged 3 commits into from
Aug 18, 2022

Conversation

DamianOsipiuk
Copy link
Contributor

@DamianOsipiuk DamianOsipiuk commented Aug 16, 2022

@TkDodo
This should fix:

  • some bundlers picking up browser entry in package.json (umd build instead of cjs or esm)
  • unusual size of react-query, due to missing rollup global definition for react-dom

This won't fix:

  • bundlers that support exports, but does not support development/production conditionals (https://webpack.js.org/guides/package-exports/#optimizations)
    This might be the case as those conditionals were mostly proposed by webpack.
    We could potentially overcome this by linking exports/./default/default not to noop.js, but to intermediate js file that conditionally require devtools based on NODE_ENV, like in the v3.
    This however i would like to test separately.

Previous beta release also have a side effect that react-query UMD build does not contain query-core UMD build and therefore is smaller. Is that something we should fix to be bundled together?

--

I would like to get this as a beta pre-release to test it out.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 16, 2022

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 8b092f9:

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

@TkDodo
Copy link
Collaborator

TkDodo commented Aug 17, 2022

I merged main back into beta - there were lots of conflicts. Can you update your PR with beta too please?

@TkDodo
Copy link
Collaborator

TkDodo commented Aug 17, 2022

then I'll make the next beta release. Current release is 4.2.0 - does that make the next beta be 4.3.0-beta.0 ?

@DamianOsipiuk
Copy link
Contributor Author

then I'll make the next beta release. Current release is 4.2.0 - does that make the next beta be 4.3.0-beta.0 ?

I would say 4.2.2-beta.0, but you can also do 4.3.0-beta.0. It should not matter too much.

@TkDodo
Copy link
Collaborator

TkDodo commented Aug 18, 2022

@DamianOsipiuk I wanted to check the bundle size, but it seems the config is no longer correct:

query/package.json

Lines 120 to 126 in 8025dbf

"bundlewatch": {
"files": [
{
"path": "packages/**/build/umd/*.production.js"
}
]
}

Not sure where the umd builds are now ? I think we just need to point towards packages/**/build/lib/index.js ?

@DamianOsipiuk
Copy link
Contributor Author

@TkDodo UMD build files should be in the same place and the config should still work.

image

@TkDodo
Copy link
Collaborator

TkDodo commented Aug 18, 2022

@DamianOsipiuk yeah I've seen the results, but they look weird, like, why is packages/react-query/build/umd/index.production.js only 2.22 kb?

@DamianOsipiuk
Copy link
Contributor Author

Previous beta release also have a side effect that react-query UMD build does not contain query-core UMD build and therefore is smaller. Is that something we should fix to be bundled together?

Basically in current rollup setup, if we mark query-core as external to strip it out from cjs and esm builds, it also strips it from umd build. Previously it was not the case.

Therefore if we want to maintain previous way of bundling react-query and query-core together but only for UMD build, rollup config needs to be restructured.

@TkDodo
Copy link
Collaborator

TkDodo commented Aug 18, 2022

ah yes. so in the docs we say that react-query can be installed via:

<script src="https://unpkg.com/@tanstack/react-query@4/build/umd/index.production.js"></script>

this should still work, so we can't really break that

@DamianOsipiuk
Copy link
Contributor Author

DamianOsipiuk commented Aug 18, 2022

Hmmm, i think that right now you would have to

<script src="https://unpkg.com/@tanstack/query-core@4/build/umd/index.production.js"></script>
<script src="https://unpkg.com/@tanstack/react-query@4/build/umd/index.production.js"></script>

So it is kind of a breaking change unfortunately 😞

I will look at the rollup config, maybe i will come up with some clever override.

@DamianOsipiuk
Copy link
Contributor Author

@TkDodo Should be good now

@codecov-commenter
Copy link

Codecov Report

Merging #4044 (8b092f9) into beta (e0aad73) will decrease coverage by 0.29%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             beta    #4044      +/-   ##
==========================================
- Coverage   97.11%   96.82%   -0.30%     
==========================================
  Files          50       57       +7     
  Lines        2391     2673     +282     
  Branches      706      785      +79     
==========================================
+ Hits         2322     2588     +266     
- Misses         67       83      +16     
  Partials        2        2              
Impacted Files Coverage Δ
src/devtools/devtools.tsx
src/core/notifyManager.ts
src/devtools/theme.tsx
src/core/query.ts
src/reactjs/useQueries.ts
src/reactjs/useIsMutating.ts
src/reactjs/useIsFetching.ts
src/devtools/Logo.tsx
src/createWebStoragePersister/index.ts
src/devtools/utils.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.

@TkDodo TkDodo merged commit 58e3ae3 into TanStack:beta Aug 18, 2022
TkDodo added a commit that referenced this pull request Sep 10, 2022
* fix: restore missing `exports` declarations (#3892)

* fix: restore missing `exports` declarations

* fix: restore package.json exports[C

* fix: reexport types used by vue-query

* fix: react-native uSES usage

* fix: emit mjs for esm

* fix: uSES build

* fix: devtools exports to allow devtools in prod

* fix: cjs and esm build bundled to lib dir

* fix: sideEffect in react-query, better files paths

* fix: generate declarations to lib

* fix: lint and tests

* fix: use the same ts build method for tests

* fix: change force prod import

* fix: subpackage dependencies (#4013)

* fix: umd-build (#3924)

* - Fix UMD build getting overwritten
- Updating "browser" field for @tanstack/react-query-devtools

* Updating the "browser" field to be the same as "main"

* release: v4.0.11-beta.0

* release: v4.0.11-beta.1

* fix(react-query-devtools): cjs devtools fallback (#4048)

* release: v4.0.11-beta.2

* fix: remove browser entry, fix umd size (#4044)

* fix: remove browser entry, fix umd size

* fix: bundle query-core with react-query for umd

* fix: remove missed browser entry

* chore: remove 'browser' field from package validation

because it doesn't exist anymore

* release: v4.0.11-beta.3

* release: v4.0.11-beta.4

* chore: react-query should be a peerDependency of the devtools

* release: v4.0.11-beta.5

* release: v4.3.0-beta.0

* fix: webpack 4 fallback to cjs (#4069)

* fix: publish script shouldn't check against module anymore

* fix: publish script

I don't think we can have single quotes in commit message when passing them to --notes

* release: v4.3.0-beta.2

* release: v4.3.0-beta.3

* fix: umd build size, force prod devtools (#4074)

* fix: umd build size

* fix: devtools force production

* release: v4.3.0-beta.4

* release: v4.3.0-beta.5

* fix: reintroduce production export (#4090)

* release: v4.3.0-beta.6

* fix(react-query-devtools): always useEffect for the mounted check

no effect runs on the server, and there is no real advantage to useLayoutEffect on the client; somehow, this dynamic check creates problems with the production build of the devtools

* release: v4.3.0-beta.7

* docs: document devtools in production

* docs: document devtools in production

* docs: document devtools in production

nodenext needs 4.7

* fix: support react-native (#4125)

* fix: support react-native

* chore: remove banner from build

Co-authored-by: Damian Osipiuk <osipiukd+git@gmail.com>
Co-authored-by: John Daly <john.daly2@gmail.com>
Co-authored-by: Tanner Linsley <tannerlinsley@users.noreply.github.com>
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.

None yet

3 participants