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

Properly setup the renderAfterDocumentEvent option #62

Merged
merged 1 commit into from
Jan 30, 2020
Merged

Conversation

icidasset
Copy link
Contributor

@icidasset icidasset commented Jan 30, 2020

@dillonkearns #61 didn't fix #40 for me, this does. It turns out the renderAfterDocumentEvent option wasn't properly set up.

renderAfterDocumentEvent should be passed as an option to an instance of PrerenderSPAPlugin.PuppeteerRenderer, not to the plugin itself.

See https://github.com/chrisvfritz/prerender-spa-plugin#advanced-usage-webpackconfigjs

@dillonkearns
Copy link
Owner

🤦‍♂ Oh wow, funny how untyped languages just let you think that your code is doing something when it's actually just throwing it away 😂

Great find @icidasset! Thank you so much for figuring that out.

I still wonder if there could ever be a race condition where Elm doesn't finish painting the view in time 🤔 But since this fixes it for you, let's just go with this for now and then we'll see if the problem seems to reliably be fixed now. Thank you for this fix! 🙏

@icidasset
Copy link
Contributor Author

Haha, totally. I feel you 😄

I still wonder if there could ever be a race condition where Elm doesn't finish painting the view in time 🤔

You could do the setTimeout(..., 0) "hack" alternatively?
That would allow the rendering engine to catch up.
More info on that here: https://stackoverflow.com/a/779785

@icidasset icidasset deleted the prerender-part-ii branch January 30, 2020 17:44
@dillonkearns
Copy link
Owner

Smart! Although it's possible that the Elm event wouldn't be queued up yet? So I'm not positive if that would fix it.

It seems like the real, robust solution would be to listen for the DOM to actually contain something that we know was rendered by the Elm view.

But for now, I published NPM version 1.2.3 and Elm package 3.0.1, so hopefully that gets you past the problem! Let me know if you notice any issues.

@icidasset
Copy link
Contributor Author

icidasset commented Jan 30, 2020

Smart! Although it's possible that the Elm event wouldn't be queued up yet? So I'm not positive if that would fix it.

Yeah, hard to say. You probably know better than me, I don't know how Elm works internally at all.

so hopefully that gets you past the problem! Let me know if you notice any issues.

Will do, thank you! ☺️

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.

2 participants