Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

Release 2.5.0 #2758

Merged
merged 59 commits into from
Feb 26, 2019
Merged

Release 2.5.0 #2758

merged 59 commits into from
Feb 26, 2019

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Jan 28, 2019

This 2.5.0 release branch/PR provides a staging area for the next minor version of react-apollo. PRs that introduce minor breaking changes should be merged into the release-2.5.0 branch instead of master, so that we can keep releasing patch updates in the meantime.

Similar PRs on other Apollo repositories:

JoviDeCroock and others added 8 commits January 20, 2019 10:55
Use our own flowRight-like function instead of using lodash.flowright.

In addition to being smaller, this implementation does not require using CommonJS require to import a separate package.
This reduces the minified size of lib/react-apollo.umd.js from 28074B to
22773B (18.9%). The minified+gzip size drops from 6930B to 6451B (6.9%).

The gzip percentage is smaller because there was some repetition between
the multiple declarations of the __extends function, and gzip is good at
compressing long repeated substrings.
* chore: optimize build time

* fix: remove comments

* tests: revert transforms

* fix tests

* remove babel key from pkg.json

* chore: apply pr remarks

* chore: undo all and add esm bundle with potential todo

* remove unused babel plugins

* cjs

* some optimizations

* add question

* chore: follow pr

* ignore rpt_cache
@JoviDeCroock @Pajn @rosskevin I'm hoping this prerelease helps with
testing the changes in PR #2661.
This shaves a tiny amount of bundle size, but more importantly it
implements shallowEqual using TypeScript, and avoids one of the few
remaining uses of require.
The shallowEqual typings became unnecessary with commit
b72dfd6.
There will no longer be a "browser" field in package.json, since the
"main" field (lib/index.js) is perfectly usable CommonJS.
Now that src/walkTree.ts is compiled to lib/walkTree.js, which is plain
CommonJS, there's no need to convert lib/walkTree.js to a UMD module. The
same goes for test-links.tsx and test-util.ts.
Since the test:compiled:server:umd script uses lib/react-apollo.umd.js,
compiling that file from the ESM bundle is a good way to test that the ESM
bundle works.
Now that dist/bundlesize.js is compiled (as CommonJS) from the ESM bundle
(lib/react-apollo.esm.js), the minified+gzip size is considerably smaller!
@danilobuerger
Copy link
Contributor

danilobuerger commented Jan 29, 2019

2.5.0-beta.0 works in my react-native builds without any regressions. (Didn't check bundle size as a few KBs is insignificant on react-native)

We don't run this script regularly, so it's no surprise that the example
tests are currently broken. The script definitely doesn't belong in the
root of the repository, though.
@benjamn benjamn force-pushed the release-2.5.0 branch 2 times, most recently from 669da80 to f9f9a67 Compare January 29, 2019 17:51
The official Facebook invariant implementation supports several features
that we do not need in react-apollo, such as message string formatting,
and runtime verification that a message string was provided.

Using this TypeScript implementation of the invariant function will
enforce that we do not use these extra features, while also allowing a
simpler implementation.

In the future, when we provide a production bundle, we will want to strip
the error strings from invariant calls, like React does.

The new InvariantError class can be thrown after manually checking a
condition in cases where it's important for TypeScript to understand the
condition (and use it for type narrowing later in the code). We can strip
the message strings from new InvariantError("...") expressions, too.
By replacing

  invariant(condition, message)

with

  process.env.NODE_ENV === "production"
    ? invariant(condition)
    : invariant(condition, message)

during the Rollup ESM bundling step, we can empower minifiers to replace
process.env.NODE_ENV with a string literal to eliminate the unreachable
branch of the conditional expression, which effectively removes error
message string literals in production, without having to build or choose
between separate development and production bundles. This optimization
removes almost 1KB of minified+gzip size from the react-apollo package.

I am somewhat worried about process.env.NODE_ENV not being defined in
browsers in non-production builds. However, I think we can supply (or
recommend) a polyfill for those cases. As a counterpoint, React relies on
the existence of process.env.NODE_ENV and does not provide a polyfill:
https://github.com/facebook/react/blob/master/packages/react/npm/index.js
@benjamn
Copy link
Member Author

benjamn commented Feb 7, 2019

@brunojdo The only known breaking change is that walkTree is no longer exported from the top-level react-apollo package. You will be able to import it from react-apollo/walkTree if you still need to use it for some reason, even though there is no way for us to make it compatible with the latest React versions, because of React hooks.

Though this is theoretically a breaking change, I do not think it warrants jumping to [email protected]. If you disagree, please consider switching to a ~x.y.z version constraint for react-apollo (instead of ^) so that you will not accidentally update to new minor versions of the library. And of course it's a good idea to use a package-lock.json file in your applications, so that you don't ever have to worry about unintentional upgrades, but I assume that's old news by now.

benjamn and others added 18 commits February 7, 2019 14:05
When testing new `react-apollo` changes out (during core
development), it can be useful to `npm link` `react-apollo`
into a test app. After the link is in place, the `npm run watch`
script can be used to compile typescript file changes, in watch
mode. So while working on the `react-apollo` codebase, .ts file
changes will be picked up automatically, compiled, and because of
the `npm link`, the test app will recognize that there have
been changes made to `react-apollo` and re-run with the updated
version.

Unfortunately, `npm run watch` doesn’t currently work as it’s
missing the additional post compilation steps that
`npm run compile` leverages via `postcompile`. This mean that
`npm run watch` isn’t calling `rollup` and generating the
associated ESM/UMD bundles. So when typescript changes are picked
up and compiled, the application at the other end of the link
doesn’t always recognize that `react-apollo` has been updated,
since it doesn’t see changes in the ESM/UMD bundles (if it’s
using them - e.g. `create-react-app` is).

This commit first introduces the `tsc-watch` helper utility to
make it easier to run scripts after watched typescript files are
compiled. It then wires in changes to call `npm run postcompile`
after watched typescript compilation has completed. This helps
ensure that apps at the other end of the link see changes,
since the ESM/UMD bundles are regenerated.

One thing to note is that running rollup with the current config
takes about 3 seconds. This adds a delay between making changes in
`react-apollo` and seeing them show up in the linked application.
We can reduce this time by adjusting the rollup config to
accommodate watching and only generate the bundles we need, but
given that this functionality is currently broken as is, this
commit will at least get things back to a working state.
The matching code for this test was removed in
cce3316.
This test was depending on polling to complete within a specific
time interval, but wasn't accommodating for extra processing time
that might be needed during that interval. Instead of relying on
a specific time frame, these changes adapt the test to finish
after 3 polling intervals have completed (and polling time has
been reduced to help this test finish in half the time that it
did previously).
For some reason polling tests that use jest timer mocks have
started failing. This is likely related to recent Apollo Client
changes that were made to improve query polling (apollographql/apollo-client@e399ad8),
and jest not being able to properly mock AC's timers. Regardless
these changes just seem to impact the way jest timer mocks work,
they don't impact the actual code that is being tested. These
tests can be written and verified without using jest
timer mocks, so this commit removes them and verifies things
using an alternate approach.

At some point we might want to look into why using
`jest.useFakeTimers()` and `jest.runTimersToTime()` no longer
seems to jive with Apollo Client query polling, but since our
use of mock timers is fairly minimal, and in each case we can
use an alternative approach to test the same end result, we
should be okay to leave this for now.

cc @benjamn
In the `Query` component, we’re watching for any data that comes
in through the Apollo Client `watchQuery` Observable (started in
`startQuerySubscription`). When new data is received via the
Observable subscription, we’re calling `React.forceUpdate` to
force a re-render of the component, so it can use the the new data.

While this works, the problem with this approach is that we’re
forcing a re-render of the component every time a new result
comes in through the subscription (excluding a small check to
avoid an initial re-render when getting the component ready),
even if the component is already in a state that matches the state
it would be put into, after using the result that just came in
from the subscription. This can lead to unnecessary re-renders.

This problem has always been lurking in React Apollo, but has
become more evident due to changes made in Apollo Client 2.5.0.

This commit updates React Apollo to keep track of the last
result that was received in an Observble subscription chain,
and will compare the latest incoming result against this last
result, before deciding if the component should be `forceUpdate`d.
Recent changes to both Apollo Client and Apollo Link have
impacted the approach used to verify that queries are re-run
properly when a Query component has its `client` toggled to
another version. This commit updates the test to verify that
`client` toggling works with current Apollo libs.
Co-Authored-By: hwillson <[email protected]>
Changes to accommodate AC's integrated local state handling
@hwillson
Copy link
Member

Hi all - the React Apollo 2.5.0 release candidate is now ready for testing. If you're interested in trying it out, update to react-apollo@next (and apollo-client@next). Thanks!

@hwillson hwillson merged commit 2a330a7 into master Feb 26, 2019
@hwillson hwillson deleted the release-2.5.0 branch June 21, 2019 18:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants