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

Don't get shop to pre-render page #746

Merged
merged 3 commits into from
Feb 15, 2021
Merged

Conversation

Akarshit
Copy link
Contributor

@Akarshit Akarshit commented Jan 29, 2021

Resolves #742
Impact: critical
Type: bugfix

Issue

The cart page was being saved on build time, which was causing the shop to null as there was no server to get the shop from during build time.
Also the all the pages were being re-built after 2 mins, because of a wrong variable check.

Solution

Now the shop would be fetched from the backend on every request. Fixed the variable check to recreate the page when the shop is not present.

Breaking changes

None expected.

Testing

  1. Start the platform using make
  2. Stop reaction_api using docker stop reaction_api_1. This would simulate the circle CI build env in which the server is not running.
  3. Build the example-storefront image using docker build --network=host -t reactioncommerce/example-storefront:test-me . in example-storefront directory
  4. Once complete get the server back up using make init-reaction
  5. Run the new image using docker run -it --name storefront -p 4000:4000 --env-file .env.prod --network reaction.localhost reactioncommerce/example-storefront:test-me. If you get an error that /storefront is being used by XXX, do docker rm XXX
  6. Login, try adding products to cart, go to cart page. Everything should work.

There are other pages that use unstable_revalidate, but I was getting very sketchy behavior on the first load using that. So I thought it might be better to remove SSR for now, and if needed people can extend and add that according to their need. The behaviour is explained in the linked issue.

Signed-off-by: Akarshit Wal <akarshit.wal@mailchimp.com>
@janus-reith
Copy link
Collaborator

janus-reith commented Jan 29, 2021

I feel like there should be a general solution to this: #718

Yes, the api would need to be reachable during build time currently.
I tried to point out some potential way to change that in the linked issue.

I doubt however that fully opting out and server-rendering per request instead is a better solution to this.

@Akarshit Akarshit changed the title Don't get shop to pre-render page [WIP] Don't get shop to pre-render page Jan 29, 2021
Signed-off-by: Akarshit Wal <akarshit.wal@mailchimp.com>
@Akarshit Akarshit changed the title [WIP] Don't get shop to pre-render page Don't get shop to pre-render page Jan 29, 2021
@Akarshit
Copy link
Contributor Author

The build is failing because of some other reason. I can see that happening in other PRs too.

Signed-off-by: Akarshit Wal <akarshit.wal@mailchimp.com>
@jrw421
Copy link
Contributor

jrw421 commented Feb 4, 2021

Hey @Akarshit - I just followed the steps listed to test and still get the same bug. Am I missing something? 🤔

@Akarshit
Copy link
Contributor Author

Akarshit commented Feb 4, 2021

@jrw421 Hmm, can't say for sure. We can do a quick screen-sharing session to sort this out...

Copy link
Contributor

@jrw421 jrw421 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and looks good! 🎉

@Akarshit Akarshit merged commit a7d6383 into trunk Feb 15, 2021
@Akarshit Akarshit deleted the fix-742-akarshit-empty-cart branch February 15, 2021 22:03
@Akarshit Akarshit mentioned this pull request Mar 3, 2021
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.

Checkout flow not functioning as expected when running in local production mode
3 participants