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

[feat]: Converge behaviour between develop and build #10706

Closed
1 task
sidharthachatterjee opened this issue Dec 28, 2018 · 22 comments
Closed
1 task

[feat]: Converge behaviour between develop and build #10706

sidharthachatterjee opened this issue Dec 28, 2018 · 22 comments
Assignees
Labels
type: feature or enhancement Issue that is not a bug and requests the addition of a new feature or enhancement.

Comments

@sidharthachatterjee
Copy link
Contributor

sidharthachatterjee commented Dec 28, 2018

A common theme amongst several issues (#10689, #10370) over some time now has been

It works during develop but build breaks

Common Scenarios

  • Styling tends to be a common one because we use a different loader for the two environments which leads to several differences (@pieh can elaborate a lot more on this)
  • React helmet and gatsby-offline-plugin
  • Access to window (or other browser only APIs) and having it break only on build
  • The biggest one by far, ReactDOM hydration issues (we don't render to html in develop and people often see issues when they build because

React expects that the rendered content is identical between the server and the client. It can patch up differences in text content, but you should treat mismatches as bugs and fix them. In development mode, React warns about mismatches during hydration. There are no guarantees that attribute differences will be patched up in case of mismatches. This is important for performance reasons because in most apps, mismatches are rare, and so validating all markup would be prohibitively expensive.

I think we need to converge the differences in behaviour between develop and build as much as possible (without breaking our great hot reloading experience) so that

  • issues like these can be caught as early as possible
  • one can trust that build will succeed without unexpected issues if develop was working fine

One action point that @pieh and I found from this is

  • Serve static html on entry point in webpack dev server (so that hydration occurs during develop as well)

Please post more and your thoughts about this

@KyleAMathews
Copy link
Contributor

I love this idea! Yeah, this would help people a ton in catching weird React SSR issues early.

This is tied too to issues like #8342

Something a lot of people try to do is run Lighthouse on their dev server and complain Gatsby is super slow and has huge JS files.

@dcorb
Copy link
Contributor

dcorb commented Dec 28, 2018

great summary of issues with same root! I agree totally on SSR during develop. But not generating all pages.

CLI should also educate more about gatsby serve, it goes unnoticed easily I have seen

But generating every of 20.000 pages, every time I change I save the CSS file will hurt badly the dev hot-reload experience.

@pieh
Copy link
Contributor

pieh commented Dec 28, 2018

@dcorb My initial idea is to lazy generate only the page that user visit which would match the behaviour of production builds where html is only really used for initial paint (further navigation is done entirely client-side with react)

@dcorb
Copy link
Contributor

dcorb commented Dec 28, 2018

@pieh has sense 💯

@jonniebigodes
Copy link

@pieh that does sound like a really, really nice approach, one that could lead to a decrease on issues regarding a particular subset.

@sidharthachatterjee
Copy link
Contributor Author

sidharthachatterjee commented Jan 1, 2019

Another issue relating to this at #10749

@sidharthachatterjee
Copy link
Contributor Author

My initial idea is to lazy generate only the page that user visit which would match the behaviour of production builds where html is only really used for initial paint (further navigation is done entirely client-side with react)

One caveat of only rendering the entry point during develop is that this will only catch errors of this type

Access to window (or other browser only APIs) and having it break only on build

for the entry point.

For example:
There are two pages, index and hello. hello.js has some code that accesses window in the constructor

  • the user starts browsing at / during develop
  • the user then navigates to /hello

Everything will work fine without warnings. The moment the user refreshes the page at /hello, it will break.

Rendering everything during develop will definitely slow down startup significantly but we need something more to warn about these

@KyleAMathews
Copy link
Contributor

We could also queue pages to render in the background. Rendering other pages isn't on the critical path. We also would only need to render one page per component. We could track file changes and re-SSR them.

@sidharthachatterjee
Copy link
Contributor Author

sidharthachatterjee commented Jan 1, 2019

Rendering only one instance of a page or template makes sense.

Queuing pages to render in the background could lead to strange behaviour where develop initially succeeds and then fails asynchronously (while the user has already switched over to the browser)

We could only warn (and not panic) on issues but I think we ought to try to keep behaviour as close to build as possible and therefore panic

@sidharthachatterjee
Copy link
Contributor Author

Another issue that could be potentially handled by this is #9911

@polarathene
Copy link
Contributor

Is there a docs page related to this topic? Some tips/advice/best practices about it would be a good reference to have.

My linked issue #11099 uses URL params to affect a component, one prop adds blur if true, another adds a div if true, the latter works but the prop that would update/modify styles doesn't.

I know that original issue comment here quotes about why that behaviour happens with how React works, but I think navigating to other internal pages of the site being client-side app will not run into the issue, so navigating back to that same page would then render correctly? So why doesn't the page re-render itself upon hydration which should fix all this? It'd be run once, so shouldn't be all that expensive and would resolve many of the problems?

@sidharthachatterjee
Copy link
Contributor Author

Is there a docs page related to this topic? Some tips/advice/best practices about it would be a good reference to have.

Not at the moment, unfortunately. I definitely see value in adding some documentation for hydration gotchas (since a lot of people have been hitting them lately).

So why doesn't the page re-render itself upon hydration which should fix all this? It'd be run once, so shouldn't be all that expensive and would resolve many of the problems?

Seems counterintuitive to me because (if I understand this correctly) it defeats the purpose of any SSR

@polarathene
Copy link
Contributor

Not at the moment, unfortunately. I definitely see value in adding some documentation for hydration gotchas (since a lot of people have been hitting them lately).

I guess this issue serves to collect those? It's a bit confusing as a new user to gatsby with it blurring the lines between static and dynamic sites.

In my case I know gatsbygram was using the URL location prop to be a bit dynamic like and use that information to render components/page in certain ways, so I thought I could do similar and it worked in develop, then confused for some time trying to pin down what was going wrong with the production build.

Seems counterintuitive to me because (if I understand this correctly) it defeats the purpose of any SSR

Isn't the SSR part what we get prior to hydration? Not sure what difference it makes at the hydration point beyond ensuring the page is "patched" up to reflect what it would when navigated to from any other page vs loaded directly via url?

@sidharthachatterjee
Copy link
Contributor Author

So why doesn't the page re-render itself upon hydration which should fix all this? It'd be run once, so shouldn't be all that expensive and would resolve many of the problems?

I might've misunderstood what you meant by this. When you say re-render itself upon hydration, do you mean discarding everything in the DOM and React rendering on a clean slate? This can be jarring after the initial HTML render.

@polarathene
Copy link
Contributor

When you say re-render itself upon hydration, do you mean discarding everything in the DOM and React rendering on a clean slate?

Yes, anything on the page that was related to React, assuming that'd fix all the issues.

This can be jarring after the initial HTML render.

Oh that's not good then :(

@gatsbot
Copy link

gatsbot bot commented Feb 15, 2019

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here.

If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!

Thanks for being a part of the Gatsby community! 💪💜

@gatsbot gatsbot bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Feb 15, 2019
@DSchau DSchau changed the title Converge behaviour between develop and build [feat]: Converge behaviour between develop and build Mar 26, 2019
@m-allanson
Copy link
Contributor

This would be a great improvement, but this issue doesn't feel very actionable right now. Is there a more focussed issue we can link to?

@davidde
Copy link

davidde commented Nov 17, 2019

@m-allanson I just filed #19563.

It documents a case where gatsby develop styles differ from gatsby build due to sass imports.
The imported variables work fine in development, but are undefined on build, resulting in inconsistent styling that only manifests itself on the first page load in production.

It includes a minimal reproduction that clearly demonstrates this problem:
https://github.com/DavidDeprost/gatsby-build--sass-bug

@gaearon
Copy link

gaearon commented Feb 12, 2020

Have you considered running SSR on the client in DEV to simulate a subset of these problems without taking a perf hit?

#17914 (comment)

@jlkiri
Copy link
Contributor

jlkiri commented Mar 29, 2020

Possibly related: #22287

@DSchau DSchau added the type: feature or enhancement Issue that is not a bug and requests the addition of a new feature or enhancement. label Jun 16, 2020
@vcarl
Copy link
Contributor

vcarl commented Jul 14, 2020

Just chiming in in support of this issue. Production-only issues have been a persistent headache on a complex site I'm building/maintaining (stellar/new-docs, particularly the API Reference content), and things like initial load optimizations have been quite tricky to test because of the lack of initial HTML served when requesting a page in dev mode. Add in the various caches and we've had a number of bugs squeak out because it's difficult to confirm behavior precisely.

@wardpeet
Copy link
Contributor

Going to close this in favor of #25729

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature or enhancement Issue that is not a bug and requests the addition of a new feature or enhancement.
Projects
None yet
Development

No branches or pull requests