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

Server-side rendering: renderToStringWithData causing react warning #169

Closed
hugoduraes opened this Issue Aug 25, 2016 · 8 comments

Comments

Projects
None yet
4 participants
@hugoduraes
Copy link

hugoduraes commented Aug 25, 2016

I'm experiencing this warning while rendering on the server side using the method renderToStringWithData:

Warning: Target node has markup rendered by React, but there are unrelated nodes as well

This is renderToStringWithData current implementation:

function renderToStringWithData(component) {
    return getDataFromTree(component)
        .then(function (_a) {
        var store = _a.store, client = _a.client;
        var markup = ReactDOM.renderToString(component);
        var initialState = store.getState();
        var key = client.reduxRootKey;
        for (var queryId in initialState[key].queries) {
            var fieldsToNotShip = ['minimizedQuery', 'minimizedQueryString'];
            for (var _i = 0, fieldsToNotShip_1 = fieldsToNotShip; _i < fieldsToNotShip_1.length; _i++) {
                var field = fieldsToNotShip_1[_i];
                delete initialState[key].queries[queryId][field];
            }
        }
        initialState = encodeURI(JSON.stringify(initialState));
        var payload = "<script>window.__APOLLO_STATE__ = \"" + initialState + "\";</script>";
        markup += payload;
        return markup;
    });
}

I believe the warning is being caused by the injection of the payload on the markup, which will end up on the target node. On the client side, when React tries to render the components on the target node, it finds an unrelated node (the script tag) and throws the warning.

I think this method should be changed so that it does not render the component but calculates the initialState and returns it instead.

What are your thoughts about this?

@hugoduraes

This comment has been minimized.

Copy link
Author

hugoduraes commented Aug 26, 2016

I've noticed you added a note about this on the documentation:
http://docs.apollostack.com/apollo-client/react.html#renderToStringWithData

I'm currently using getDataFromTree directly, but when do you plan to have a fix?

@jbaxleyiii

This comment has been minimized.

Copy link
Member

jbaxleyiii commented Aug 26, 2016

@hugoduraes yes! We are still thinking through the best way to make this useful. Currently we are thinking renderToStrinhWithData.then({ markup, initialState }) => ...

@hugoduraes

This comment has been minimized.

Copy link
Author

hugoduraes commented Aug 27, 2016

That's perfect. It solves the problem and it's a simple fix. Looking forward to it. :)

@tmeasday

This comment has been minimized.

Copy link
Contributor

tmeasday commented Sep 1, 2016

@jbaxleyiii should I just implement this?

@jbaxleyiii

This comment has been minimized.

Copy link
Member

jbaxleyiii commented Sep 1, 2016

@tmeasday I've got it done in a local branch. I'll push it up!

@jbaxleyiii

This comment has been minimized.

Copy link
Member

jbaxleyiii commented Sep 1, 2016

@tmeasday #184

I'm planning on finishing a few bugs and doing a release on friday fwiw

@zol zol removed the in progress label Sep 5, 2016

@hugoduraes

This comment has been minimized.

Copy link
Author

hugoduraes commented Sep 5, 2016

@jbaxleyiii Cool! :) When do you plan to make a release containing this?

@jbaxleyiii

This comment has been minimized.

Copy link
Member

jbaxleyiii commented Sep 6, 2016

@hugoduraes today is the plan!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment