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

Support getDerivedStateFromProps #2076

Merged
merged 6 commits into from Jun 26, 2018

Conversation

@amannn
Copy link
Contributor

commented Jun 6, 2018

In contrast to componentWillReceiveProps, the new getDerivedStateFromProps lifecycle method is also called before the first render.

It looks like you have some kind of script set up that fixes indentation before a commit. I therefore made two separate commits, so it's easier to differentiate the actual change from the diff.

Checklist:

  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests
  • If this was a change that affects the external API used in GitHunt-React, update GitHunt-React and post a link to the PR in the discussion.
@amannn amannn referenced this pull request Jun 6, 2018
1 of 3 tasks complete
Copy link
Contributor

left a comment

This looks good to go! I appreciated the separate commit for indentation.

@amannn

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2018

@mike-marcacci Thanks for the review! Can this be merged then?

@trojanowski

This comment has been minimized.

Copy link
Contributor

commented Jun 11, 2018

@amannn I think one change is required - if getDerivedStateFromProps is defined then componentWillMount shouldn't be called. Please look at https://github.com/facebook/react/blob/a2cc3c38e214c16ff6805312d4353c1faab9ff95/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js#L660 and https://codesandbox.io/s/x93872vmyw.

@amannn

This comment has been minimized.

Copy link
Contributor Author

commented Jun 12, 2018

@trojanowski Good point! I've updated the PR accordingly.

@marnusw

This comment has been minimized.

Copy link
Contributor

commented Jun 25, 2018

@hwillson this is a follow-up issue to the recently merged #1978 which will make SSR fully compliant with React 16.3 (as far as I can tell). Is there any chance this PR could be fast-tracked for release in the near future?

@hwillson hwillson self-assigned this Jun 25, 2018
Copy link
Member

left a comment

Looks great @amannn - thanks very much!

@hwillson

This comment has been minimized.

Copy link
Member

commented Jun 26, 2018

@marnusw Merging it now, which means we should be able to get this released tomorrow.

@hwillson hwillson merged commit 0ed2d4a into apollographql:master Jun 26, 2018
11 checks passed
11 checks passed
CLA Author has signed the Meteor CLA.
Details
bundlesize ./dist/bundlesize.js: 9.98KB < maxSize 11KB (gzip)(31B larger than master, careful!)
Details
ci/circleci: Browser Your tests passed on CircleCI!
Details
ci/circleci: Commonjs Your tests passed on CircleCI!
Details
ci/circleci: Filesize Your tests passed on CircleCI!
Details
ci/circleci: Linting Your tests passed on CircleCI!
Details
ci/circleci: Node.js 8 Your tests passed on CircleCI!
Details
ci/circleci: Node.js 9 Your tests passed on CircleCI!
Details
ci/circleci: Preact Your tests passed on CircleCI!
Details
ci/circleci: Server Your tests passed on CircleCI!
Details
ci/circleci: Typecheck Your tests passed on CircleCI!
Details
@marnusw

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2018

Thank you @hwillson! We appreciate your time and that you jumped on this so quickly.

And thanks @amannn for the fix.

@hwillson

This comment has been minimized.

Copy link
Member

commented Jun 27, 2018

This is now available in react-apollo 2.1.7 - thanks all!

@zsolt-dev

This comment has been minimized.

Copy link

commented Jun 27, 2018

Thank for this bug fix... Was waiting for this for some time.

But now I get "loading" on SSR everywhere. Each server side rendered page return loading in the source code and empty apollo state. Data is loaded fine after page loads in the browser.

any ideas how to fix?

@tab00

This comment has been minimized.

Copy link

commented Aug 13, 2018

Even with the latest react-apollo (2.1.11) I still get the error (with <React.StrictMode>):

Warning: Unsafe lifecycle methods were found within a strict-mode tree:
    in div (created by App)
    in div (created by App)
    in div (created by App)
    in ApolloProvider (created by App)
    in App

componentWillReceiveProps: Please update the following components to use static getDerivedStateFromProps instead: Query

Learn more about this warning here:
https://fb.me/react-strict-mode-warnings
@onpaws

This comment has been minimized.

Copy link

commented Oct 4, 2018

Having the same experience as @tab00.
Do I need to opt-in somehow?

import React, { unstable_AsyncMode as AsyncMode } from 'react';
import { Query } from 'react-apollo';
import gql from 'graphql-tag';

const GET_SHIPS = gql`
query {
	allStarships {
    id
    name
    passengers
    costInCredits
    consumables
    createdAt
  }
}
`

export const Ships = () =>
  <div>
    <AsyncMode>
      <Query query={GET_SHIPS} asyncMode>
        {({ loading, error, data }) => {
          if (loading) return <h1>Loading...</h1>;
          if (error) return <h1>Error :(</h1>;

          return data.allStarships.map(ship =>
            <Ship key={ship.id} {...ship} />
          );
        }}
      </Query>
    </AsyncMode>
  </div>


const Ship = (props) =>
  <div>
    <h4>{props.name}</h4>
    <p>Passengers: {props.passengers}</p>
    <p>Cost: {props.costInCredits}</p>
    <p>Created: {props.createdAt}</p>
  </div>

React v16.5.2
macOS 10.14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.