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

Snapshot of async re-render (setState) #360

Closed
pedroBruno7 opened this issue Apr 8, 2019 · 11 comments
Closed

Snapshot of async re-render (setState) #360

pedroBruno7 opened this issue Apr 8, 2019 · 11 comments

Comments

@pedroBruno7
Copy link

Hi,

Basically my question is the same I made here geelen/react-snapshot#122 on react-snapshot project. In your case, you seem not to have this kind of config:

"reactSnapshot": {
"snapshotDelay": 5000
}

My issue I think is also related to this: #350
I think is partially answered but the point is, is it possible to make the snapshot happen after x time and/or wait for all the page loads? This would solve a lot of problems (better SEO) and would be easy to implement (I assume).

Thanks

@stereobooster
Copy link
Owner

There is no need for this. Give it try and you will be surprised that it actually works for this case. The secret sauce is networkidle0 (consider navigation to be finished when there are no more than 0 network connections for at least 500 ms).

@pedroBruno7
Copy link
Author

pedroBruno7 commented Apr 8, 2019

There is no need for this. Give it try and you will be surprised that it actually works for this case. The secret sauce is networkidle0 (consider navigation to be finished when there are no more than 0 network connections for at least 500 ms).

Thank you for the reply @stereobooster : ) Is there a quick config to do that with react-snap, something like this #199 or are you saying I have to use Puppeteer directly (the node.js API)?

I tried the first option (with his config) and it did not work with a fetch (inside componentDidMount)

@stereobooster
Copy link
Owner

stereobooster commented Apr 8, 2019

Do you have actual problem? Did you try to use react-snap and you have problems? If yes then what the problem?

@pedroBruno7
Copy link
Author

Do you have actual problem? Did you try to use react-snap and you have problems? If yes than what the problem?

Yes, I want to pre-render a page that has some content that is fetched from an API, for SEO purposes.

And my question was: wouldn't this be easily solved with react-snap and would be an improvement if we just had an option to the delay the snapshot x seconds and / or wait for every network request on that page to finish?

I also wanted to be sure if that was already possible with react-snap
SSR involves doing a lot of changes to the code, having a lightweight option like this would be cool

@stereobooster
Copy link
Owner

Theoretically it supports that case. In most cases it is zero-changes to the code, except those that described in Readme

@pedroBruno7
Copy link
Author

Theoretically it supports that case. In most cases it is zero-changes to the code, except those that described in Readme

Are you saying react-snap has something like this? (not on the docs)

"reactSnap": {
    "puppeteerArgs": ["--no-sandbox", "--disable-setuid-sandbox"],
    "puppeteer": {
      "waitUntil": "networkidle0"
    }
  }

(tried this config and it didn't work)

Or would I have to go and work with puppeteer node.js API directly?

@stereobooster
Copy link
Owner

stereobooster commented Apr 9, 2019

react-snap already uses networkidle0. You need puppeteerArgs only if you use docker or similar.

(tried this config and it didn't work)

What didn't work? Can you report issue clearly? Like this is my config..., I run this command..., I get this output..., but I expected...

@pedroBruno7
Copy link
Author

pedroBruno7 commented Apr 14, 2019

react-snap already uses networkidle0. You need puppeteerArgs only if you use docker or similar.

(tried this config and it didn't work)

What didn't work? Can you report issue clearly? Like this is my config..., I run this command..., I get this output..., but I expected...

Sorry for the delay. Info:

Project: simple create-react-app boilerplate with a small node/express server
Setup: A node.js server on Heroku (that why I need puppeteerArgs according to the docs)

part of package.json:

"scripts": {
    "start": "react-scripts start",
    "build": "react-scripts build",
    "test": "react-scripts test",
    "eject": "react-scripts eject",
    "postbuild": "react-snap"
  },
  "reactSnap": {
    "puppeteerArgs": [
      "--no-sandbox",
      "--disable-setuid-sandbox"
    ],
    "puppeteer": {
      "waitUntil": "networkidle0"
    }
},

Command: Heroku is set to run npm run build like usual on create react app
Expected: When I push to heroku, everything works fine, it crawls the routes, create the static files with the right code. Except when I just use setState inside a fetch, inside a component did mount (for example). In that case, everything is fine except that the fetched content does not appear on the snapshot, he misses it. (not just on heroku, on dev as well)

The code is here https://github.com/pedroBruno7/reactSnapshotTest

@stereobooster
Copy link
Owner

You have following code

            axios("/api/users")
            .then((res) => {
                const data = res.data
                // console.log(data)
                this.setState({data: data.users})
            })
            .catch((err)=> {
                console.log(err)
            })

I don't know what is wrong with this code, but it hides error. If you would switch to fetch error would become obvious immediately

            fetch("/api/users")
            .then(response => {
                return response.json()
            })
            .then((data) => {
                this.setState({data: data.users})
            })
            .catch((err)=> {
                console.log(err)
            })

if you run react-snap build you will see

$ react-snap
✅  crawled 1 out of 4 (/)
💬  console.log at /users: SyntaxError: Unexpected token < in JSON at position 0
✅  crawled 2 out of 4 (/users)

the problem is that /api/users is html document, not JSON response from API. You need to use either full URL, like https://example.com/api/users or write some custom code which will use different server depending if you use react-snap or not. Also see #92 (comment)

@pedroBruno7
Copy link
Author

pedroBruno7 commented May 8, 2019

@stereobooster I'm sorry, I only saw the fork, didn't see this comment.

I understand what is happening. Puppeteer is requesting to a different URL and is not fetching correctly, it's fetching an HTML page. When I try with https (full url) it gives CORS because Puppeteer runs in a different localhost.

Could this be solved by defining the base URL for Puppeteer like you have in the docs?

const BASE_URL =
  process.env.NODE_ENV == "production" && navigator.userAgent != "ReactSnap"
    ? "/"
    : "http://xxx.yy/rest-api";

Given that is no documentation on this, can you give a fix on this or improve the original documentation to show how to solve this create react app / proxy problem?

this #92 (comment) did not work or I did no set it correctly. Is there any documentation on these package json options inside "reactSnap"?

Big thanks

@stereobooster
Copy link
Owner

#92 (comment) didn't work because this is just an idea it is not implemented.

You can do something like this

const BASE_URL =
  process.env.NODE_ENV === "production" && navigator.userAgent !== "ReactSnap"
    ? "http://development.com"
    : "https://production.com";

 fetch(`${BASE_URL}/api/users`)...

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

No branches or pull requests

2 participants