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

Run only once on server #54

Open
oyeanuj opened this issue Feb 24, 2018 · 3 comments
Open

Run only once on server #54

oyeanuj opened this issue Feb 24, 2018 · 3 comments
Labels

Comments

@oyeanuj
Copy link

oyeanuj commented Feb 24, 2018

Hey again @ctrlplusb, I was wondering if you think there should be a way for a job to run only once on the server (and not again on the client). Testing with some pages, I realized that for search engines, I was getting the LoadingComponent even when the job was completed on the server. It seems that code does get into withJob and the jobState.completed is not set to true on the client. So, it needs one additional pass to process some custom logic that I provided in the workDefinition to ensure that the jobState is considered complete.

Thoughts on if it would be possible to remember jobState from the server when evaluating the work on client?

@oyeanuj
Copy link
Author

oyeanuj commented Apr 8, 2018

@ctrlplusb On further investigation, it seems that the job is re-running because the window.jobState is empty:

window.__JOBS_STATE__={"jobs":{}}

This seems to happen if the job is resolved synchronously, as the this.context.jobs.register line below never gets called:

this.context.jobs.register(id, { data })

That is not a problem on the server-side but on the client side, it will try to re-evaluate the work definition, which means there will be a render of just the loading (at the time of mount). Unfortunately, that breaks the experience for SEO, since that's what the crawler sees even though it gets fixed in the next render phase.

I think the best way to fix this bug would be to consistently register job result regardless of whether it is synchronous or asynchronous (not sure about the side-effects of that)

Here is how that looks in my fork:
oyeanuj@5a9d5ce

If this looks good, I'd be happy to make a PR!

@ctrlplusb
Copy link
Owner

Hey @oyeanuj

Great catch! Yes, please can you create a PR.

👍

@ctrlplusb
Copy link
Owner

FYI it may be best to pull the latest version of the library and make your change. I changed up the build process quite a bit.

@ctrlplusb ctrlplusb added the bug label Aug 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants