-
Notifications
You must be signed in to change notification settings - Fork 10
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] RFC/Issue 115 Build Time Data Injection #251
Conversation
0da875a
to
560f303
Compare
Now we have working rehydration, except it's not completely working. We are storing apollo's cache into memory and then reading it before any more queries are made. That's great, it works when we fetch a new page. However, when we fetch a page using our redux-router, we're not fetching a new page, instead we're rendering components conditionally based on a route within javascript. This causes a problem because our js bundle doesn't have our pre-rendered cache. It can however, retrieve the pre-rendered cache from the initial page fetch. But in our case, that would contain the wrong query, for the previous page that we've since navigated away from. You can test this by navigating to an initial page, check the In development, it works as it should(because graphql is running on server). In production only when serving this you will see the rehydration issue. |
The solution here is that we need to rehydrate our queries, on route change, from static json files(as we have no graphql server in production for a static site), that we've already pre-rendered server side. We also need to rehydrate from the pre-serialized cache on the initial page fetch. So what we're doing is, during build we're writing static json files to our public directory when a query is received. We're also serializing that query so that its available immediately within the html page when it's initially fetched by the client. When a client navigates to a new route, we're rehydrating by fetching the cache from a .json file we wrote earlier during build. In this solution, the cache is in 2 parts. Once initially serialized as part of build. Second cache is written and fetch from file on route change. Going forward, I'd like to refactor and modularize this so that this example can be extended for other applications without the large amount of code currently needed. |
A few problems,
Logic within the page-template.js needs to be tweaked slightly. At least it's fetching the static cache, albeit potentially too often for this use case. These problems are noticable due to the slight delay before menu is rendered on initial page fetch. |
760b67d
to
9ae4db8
Compare
c8c9538
to
bde6511
Compare
it needs even more work to the logic of During serialization, our query must pull from the graphql server at least once. After querying, add the graphql cache to the DOM within a script element so that its written to the final html file. This only needs to happen during build, so within our component we need to make sure the condition to do this is appropriate. In production:
Doing all that conditionally is a royal pain in the arse. |
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.
Thanks for taking a pass at this, it's definitely a tricky challenge.
I do like the syntax that something like GraphQL brings to the table, but the implementation of which is a bit involved it seems. 😳
My initial thoughts on this were a bit more simplistic, I suppose, but I think it would work reasonably well enough and not have to "intrude" too much into user-land, so to speak. Given my snippet in the RFC description
import { graph } from 'xxx';
setupShelf() {
// map over the graph and create the shelfList object dynamically
graph.pages.map((page) => {
// xxx
}
}
xxx
would just be a JSON object (file or otherwise, e.g.)
{
"graph": {
"pages": [{
"name": "home",
"path": "/home"
}, {
"name": "about",
"path": "/about"
}]
}
}
I was thinking of using something like webpack's resolve
as the mechanism by which to do something like this for us. l'll plug away at this approach a little more will try and get a POC of it up in time for our next meeting. Being able to define a schema like GraphQL would be pretty awesome though, so I'll see if I can marry these two concepts. 🤞
(Maybe this could give rise to another use case for, or extension of, having webpack be able to input / output from memory?)
In the meantime, definitely continue on your work here and let's plan the bulk of our time during our next meeting to show and tell our approaches. ✌️
@hutchgrant |
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.
So definitely some interesting stuff going on here, but I think introducing GraphQL may have been a bit premature.
In general, the workflow I had in mind was one where the user uses import
from their code, and they get the graph
. From there, they should be able to access all the data and build their own components / UI structures, e.g.
- list of pages (e.g. blog posts)
- navigation menus
<title>
/<meta>
I think all the menu specific stuff I am seeing is a bit too specific at this point since it seems to have introduced a bit too much coupling / userland bleed than I would have expected, given the use cases defined above, e.g:
- menu in front matter
- menu in the graph
- queries
I wouldn't rule out allowing users to use things like map
, filter
, and reduce
over the graph to derive the data they need.
Maybe we try an example without GraphQL, and just resolve import { graph } from '@greenwood/data'
to that JSON file for now? I am not against using something like GraphQL, mostly for providing a nice DSL as a DAO, but it's not quite clear yet if we understand the right boundaries. I think it would be great to try something a bit simpler, even if it requires a little more boilerplate on the user side, it can show us where the pain points are and what is cumbersome, and then make sure we solve that problem correctly.
Maybe we could make some queries that would be more like "prepared statements" that we can provide for convenient / common things one might need from our graph, but I don't think we're at that level yet, and I think a couple steps away still from GraphQL. (with all the implications that brings, good or bad).
I think just building up the simplest implementation first can give us a valuable reference point to review and compare against the implementation here.
I would still recommend some clean up to this PR just make it a bit more easier to compare against other implementations, and so I would try and collapse as much of the code as possible under @greenwood/cli
, in libs/, lifecycles/, etc since I think it would all need to be "core" code ultimately.
@@ -1,3 +1,8 @@ | |||
--- | |||
menu: about |
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.
making a note of userland
bleed
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.
This is the simplest way to link a page directly with a menu.
Otherwise, you need to create seperate menu files, which we wanted to avoid to begin with, as we already do that. That would defeat the purpose of compiling those menus at build time.
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.
#115 (comment) You also suggested this yourself
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.
Ah, yeah. Those weren't meant to part of the implementation / API, just metadata about the page itself that should be passed through (if possible) to the graph when pulling the graph data.
@@ -25,11 +25,15 @@ | |||
"@babel/core": "^7.6.0", | |||
"@babel/preset-env": "^7.6.0", | |||
"@webcomponents/webcomponentsjs": "^2.3.0", | |||
"apollo-boost": "^0.4.4", |
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.
What's the relationship of all these dependencies. Do we need them all?
- graphql
- apollo
- axios
- cross fetch
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.
axios is for the ajax request, to get and hydrate from our server rendered cache on route change(when a route doesn't already contain the cache). cross fetch is a peer dependency required by one of the apollo libraries. graphql is of course a requirement of apollo.
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.
ok, thanks. Will take a little more look into that side of things.
…ProjectEvergreen/greenwood into rfc/issue-115-build-time-data-injection
…fc/issue-115-build-time-data-injection
I've got it working, albeit its not initializing from cache properly on initial page load(it's fetching cache rather than using the serialized one). Of course I need to include the other submenus in the compilation and query, as well as add special conditions so that that the menus display properly on all the pages besides main navigation pages. But again menus is just one example. We can build it in a modular way so any developer could add anything they want to the graphql server and query it via plugins etc. For now I'm only querying vars that I've added to each page's front-matter, again as an example. Also See lit-apollo for example |
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.
Good chat tonight @hutchgrant !
To reiterate, this is definitely a great start on this work and I think provides a good foundation to build up our technical design from, in terms of what we both envision as being the simplest developer experience as we can achieve, as close to the examples we came up with.
Below are just some of my high-level thoughts coming out of my further review of this work and our discussion. Most of this I think we agreed with but just wanted to document it for posterity.
Connected Components
I think the promise of a lit-apollo as you found is very promising. I think if we can stuff as much of the boilerplate into app-template.js and let page templates or components just "connect" to the Apollo state would really nicely emulate the react-redux paradigm, which I always thought nice.
So combine that with gql
and I think we're much closer to delivering a great workflow!
Derived State
To the point above, an important best practice with redux and stores in general, is try and keep data normalized and keep the store as simple as possible, which in our case, would be the graph. From there, components or templates can connect to get the subset of state they need, like for pages or subpages on their own.
I think the menu
that was added to the markdown should be injected into the process in the same way that NodeJS would inject the path of the script when using __dirname
, for example. In this way, we can promote the queries you have, but just leave out menu
as anything the CLI / compilation needs to know about.
We (Greenwood) could provide a package of common queries for things like (just a thought)
- nested menus (like in the shelf)
- navigation bar (like in the header)
- list of pages (like all pages/blog/*)
In the last example, but being able to pass through something arbitrary like this in markdown
date: 2019-11-19
Someone could then sort
the blog posts on their own.
Misc
- consolidate code into
packages/cli
- be more discriminating with our introduction of dependencies
- we will need to start thinking about how / where to document this for users
I'll start tinkering with this now that i was able to get more insight and feedback from you. Getting there!!! 💪
…fc/issue-115-build-time-data-injection
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.
Thanks for all your good work on this @hutchgrant. 🙏 🎉
Closing this in favor of #269.
Related Issue
#115
Summary of Changes
Added apollo graphql server to be run before pages are serialized/compiled in development/production. Populated by our graph.
Added graphql schemas for menus and items
Modified all markdown pages to contain menu front-matter var
Modified graph to include all menu front-matter for each page
Added seperate plugin for graphql, although it's still part of the CLI for now
Modified
page-template.js
to perform a graphql query for each menu based on the URLProblems
1. Big problem is it isnt serializing in production. For now it's a work in progress that functions in development mode only.Second big problem is I need to prevent it from attempting to rehydrate the graphql query, as the server will not be accessible once built in production.
The menu items are in alphabetical order(thats the way they're added to graph). This could be mitigated with numbered variables within front-matter and then sorted.
The menu subitems aren't being parsed and are therefore absent. We could use remarkable to parse the markdown and add sub items to the graph whereas before we did it manually.
Notes
This is just an example of how graphql could be used to inject our graph or other data sources into our compilation
Plan
1. Get this small example to serialize in production by running line by line debugs of the serialization lifecycle2. Get this small example to stop rehydrating after its already been serialized - partially working
3. Standardize and modularize then document.