-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
WIP: Add async SSR data fetching #2423
Conversation
Deploy preview ready! Built with commit c8a756c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great! This is exactly how I would have done the change :-)
One thing that I'd ask be added — a perf improvement that's coming soon to Gatsby is the ability to skip rendering pages that haven't changed. The way we can do that is through our data layer as we know what every page's data requirements are so it's easy then to say that this piece of data changed so only re-render these two pages.
This async model breaks that as then Gatsby doesn't know about how pages are changing. As we really want/need the above optimization for larger sites — let's make async SSR opt-in.
What I'm thinking is we add a field (e.g. asyncSSR
) onto the page
object so that when rendering a page, static-entry.js knows to use the async runner instead of the sync version.
You could add asyncSSR
when creating a page with createPage
or modify auto-created pages to add it.
How's that sound?
key="webpack-manifest" | ||
dangerouslySetInnerHTML={{ | ||
__html: ` | ||
.then(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use async/await here?
I like it! Thanks for the comments and ideas, makes sense. Let me see if I can do this without blowing up the entire repo. 🥇 Hopefully I'll have up later today, this weekend at the latest. |
Deploy preview failed. Built with commit 3779a19 https://app.netlify.com/sites/using-glamor/deploys/59e10a8b0b79b7400d06284e |
Deploy preview failed. Built with commit 03838d6 https://app.netlify.com/sites/using-glamor/deploys/59fa5b7fa6188f1da7f85725 |
I think I got it! For Git housekeeping: I'm a solo developer, so I'm terrible with git. I had a merge conflict and I think I fixed it..? I tried to rebase after I sent the updated PR, but I just blew up my local repo more. It looks like the code is right. If not, please let me know... To save my sanity, it would be easier for me to close this PR, I'll fork it again and reapply my changes and submit a new PR. I see the deploy preview failed above. It looks like there was an issue spinning up a container..? Onto the substance... I got static-entry to work for both asyncSSR = true and asyncSSR = false pages! I also updated the snapshot tests to add asyncSSR. On the api changes, I renamed api-runner-ssr functions to I turned static-entry into an async function. My testing and reading of the JS spec is await will work for non-promisfied functions too, so the following should be valid...
The other change is a little controversial. For Please let me know what else I can do. I'm happy that I was able to implement the feature! |
I'm not sure if this PR is failing the deploy build. Above it says it is, but below it says that its passed. It also says it failed the windows build If anyone can help me understand where to start and the changes needed, I'm happy to make them! |
@LawJolla that was fixed fairly recently so once you've resolved the yarn.lock error you'll be fine. BTW, sorry for the slow merging of this — it's a big change and I haven't had time to do a proper think of its implications :-( |
…r-async # Conflicts: # packages/gatsby/src/redux/actions.js # yarn.lock
I guess I should have tried to merge instead of rebase. 😪 What a mess. I'm so bad at git. If this is ok, great. If not, please feel free to close it and I'll PR a new fork. It looks like the builds/tests are failing for reasons unrelated to my commits. |
Deploy preview ready! Built with commit fc4f9de |
# Conflicts: # yarn.lock
Why is the deploy preview called “using drupal”? |
@sedubois we build all the example sites on every PR including using-drupal. As integration tests basically. |
@LawJolla this looks like something I could really make use of. Is there anything I could help with to push this along and get it merged? |
@imjared Huge thanks for the offer and interest! It looks like the tests are failing because the snapshot tests need updating to include Before I destroyed this PR with my rebase mess 😄 , @KyleAMathews mentioned that the PR is a big change and he needed more time to consider its consequences. But more interest in this PR helps! And I'm sure there will be more changes, so I / we would greatly appreciate the help if Kyle decides to move forward. |
I'm VERY interested in this and would be interested in contributing - I'll be anxiously returning to see the current status. |
@KyleAMathews asked me to share my use case here as I was asking for option to use async The original mission is to enable fast previews, when editing WordPress posts. And by thinking about solutions I had two ideas. But these ideas dont actually solve the previews issue 😁 afterPageQueryPossibility to define some callback to modify graphql result after // src/templates/page.js
export const pageQuery = graphql`...`;
export const afterPageQuery = async (graphqlResult, pageContext) => {
// do some magic, even async
graphqlResult.customized = true;
return graphqlResult;
}; For example I would like to transform the resulting structures of getInitialPropsThen I was thinking - why not just give the developer the option to have custom provider for the static data. If they know what they are doing. When you are ok with not using graphql. (naming taken from next.js). // src/templates/other-page.js
export const getInitialProps = async (defaultPropsFromGatsby, pageContext) => {
// do some magic, even async
defaultPropsFromGatsby.customized = await someAsyncDataLoader(pageContext.id);
return defaultPropsFromGatsby;
}; solving WordPress previewsThings above wouldnt help in solving the previews issue. But my thinking was, that we could prepare the static data for pages on the WordPress server. Then |
My first thought was that it would be great not to increase the API surface, and why bypass the GrahQL layer? It sounds like the strong point of Gatsby and bypassing it looks like a workaround rather than a fix? |
Anything new on this discussion, I am very interested in this topic for a handful of projects? |
@GerritVK It looks like Kyle and Apollo are working together for a more comprehensive solution. Kyle has the laudable goal that Gatsby tracks which pages need rebuilding, rather than an entire site rebuild. With my PR, Gatsby can't know about the Apollo data. (And I don't know how to implement it without more Gatsby API changes, like building the @sedubois It's not a work around. Gatsby doesn't manage data at runtime, by definition. If I have a realtime dashboard that's also statically generated, then I need to know what data that page requests at build time. I can duplicate Gatsby queries and client queries (Apollo/Relay/etc) and hope to stitch them together at runtime, or I can get the The downside being the data is duplicated (in the HTML and in the JS store) |
Hi @KyleAMathews! When you get a moment, I'm hoping to get your thoughts on the following. I've gotten a few comment on my approach that I'm "misusing" Gatsby when I really should be doing SSR. I have my own feelings, but what do you think -- should this style of "app" be relegated to SSR? |
@KyleAMathews is there ongoing work on gatsby-plugin-apollo as mentioned by you in one of your previous comments? Thx. |
I have a e-commerce site that is using Gatsby to render all the product pages. I'm using Apollo on the client and during runtime I fetch dynamic data like current inventory. I also need client-side search using Ideally, I'd like to pre-poulate my Apollo store with all my products so I can have client side search without making an additional query for all items. I'm already querying all the data to build the nodes and pages in Gatsby so it would be great to use those queries to hydrate local state. A temporary solution I've come up with is having a pre-build step that will create a new
It would be great to have an out of the box solution for this since the data is all there during the build process. |
@shwanton it sounds like we're doing the same thing... I like your approach because it end runs Gatsby entirely where mine requires a PR/fork. What are you doing for your Gatsby queries? My Gatsby queries are just enough to generate routes, e.g. find the products, create the slugs for the products, and then make the nodes. The data for those product pages comes from the Apollo tree walk. |
Not ongoing but it's planned either for the v2 release or during the v2 release cycle. |
@LawJolla I'm using data from a small graphql server I build over the square rest api. I use a In my
My pre-build step will use this as the only child component with the same Apollo client as my app.
The weird part is my page templates are querying the Gatsby data by A thing it took me a bit to figure out was I was imagining this could all run as part of a Docker image that could run the pre-build step so it's available to I'll try to extract this all into a working demo. |
@LawJolla Thank you for this awesome work. |
Hi @MathieuDelafosse , Thanks for the kind words and interest. I haven't heard anything on this PR in some time and I think it's still here just for a discussion thread. I'm getting the sense that our dynamic page use case is not where Gatsby wants to go and we'd be better off with an SSR package like NextJS. But there's still a place for moderately dynamic Apollo sites using Gatsby that would benefit from a Gatsby-SSR query/compile step. |
@LawJolla In order to clean up the PR list I'll close hanging PRs like this over the next few days. Can you capture problem this PR is/was trying to solve as Issue and add link to discussion in this PR there so it doesn't get lost? |
Is this PR still active? I'd love to see it get merged. |
I'm going to close this as it has a lot of conflicts that will need tidying up. Would be happy to see discussion about this carried on in a new PR or issue 👍 |
@m-allanson this PR was mergable for months but ignored by Kyle. Ruined my opinion of the Gatsby project, so I switched my company to NextJS and have no further interest in this PR or any contribution. Good luck getting your free socks everyone! |
Hey @LawJolla. Sorry about this. I should have closed this a long time ago — I just dithered because in many ways your PR is a really good idea and you put a ton of time into it and really pleased about it with your blog post, etc. Also what you tackled is a real problem with Gatsby that we still don’t have a great solution for so I kept thinking that perhaps I’d warm to your work. But that was unkind and I should have been decisive. I'm really sorry about that as I'm sure it hurt personally and caused troubles for you deciding what technical direction to take. We'll do better in the future. |
I’m also looking to any followup to this PR with great interest. I also think @LawJolla did an amazing job and I hope the ideas presented here will soon bear fruit in another form. Meanwhile I wish all the best to the Gastby team for the v2 release! |
Thanks @KyleAMathews I really appreciate that. I also apologize for being overly salty. Never post when you first wake up. |
Hi, is there any more progress on this topic? We also have some requirements around fetching data during the SSR process and being able to use async/await with the |
I'm sorry for bumping this up again, but is something along this lines on the roadmap? This would seriously improve some use-cases. I could make a gatsby plugin for ssr with I'm using an external |
The StencilJS community is also really interested in the PR! Stencil offers a |
My team is also adopting Stencil based WebComponents. The extent to which we use Gatsby going forward for static sites heavily depends on support for async SSR. My team is pretty big, with a lot of Gatsby fans. It would be a shame to go elsewhere due to this. |
@BrunnerLivio, @Jefftopia -- Here's a way to SSR Stencil components without needing to use Gatsby's |
@jonearley Brilliant, thanks so much! |
This is my hack, for lack of a better term, to allow data fetching in gatsby-ssr.
Code: I used the async function from api-browser-ssr and then wrapped static-entry in a
then
after the promise-fiedreplaceRenderer
.Purpose: This allows my application to walk my client's tree (
bodyComponent
), look for queries, and run those at compile time. Here's my gatsby-ssr for reference.https://gist.github.com/LawJolla/5fa01d54c64f1b17eea498b68068acab
You can read more about my use case here: https://medium.com/@dwalsh.sdlr/gatsby-apollo-graphcool-netlify-the-webs-promised-land-6dd510efbd72
My application is an ecommerce site. gatsby-node runs queries to determine the pages and gatsby-ssr runs the client queries to determine the data for those pages. Then when Apollo Client loads, its store hydrates and all statically generated data is now controlled by the Apollo store, including new queries and web socket subscriptions.
Huge win!