Reimplement getDataFromTree using ReactDOM.renderToStaticMarkup. #2533

benjamn · 2018-10-26T01:20:59Z

The getDataFromTree function is used during server-side rendering (and sometimes also on the client) to await all fetchQuery promises generated by Query components in a given tree of React components, so that subsequent ReactDOM.renderToString and/or renderToStaticMarkup calls can render the tree synchronously, without worrying about fetching data.

Until now, getDataFromTree was implemented by walking the component tree, instantiating components along the way, fetching any data for Query components, calling render() methods, and recursively processing children. This simulated rendering process was similar to the official rendering process performed by React, but it was not exactly the same, so it created a nontrivial maintenance burden for react-apollo to accommodate any new React features.

The recent introduction of Hooks is just one such feature, but hooks are particularly troublesome for getDataFromTree, because they depend intimately on internal React state that cannot be manipulated using any stable, public API. In other words, updating our tree-walking implementation of getDataFromTree to support hooks would have been very difficult, if not impossible. Even if it was possible, it would never be truly future-proof, since there could always be other new React features that threatened doom for getDataFromTree.

In general, the only way to make getDataFromTree truly future-proof is to implement it using the official React rendering functions, and/or wait for React to implement direct support for asynchronous server-side rendering (which would be AMAZING).

For now, this PR implements getDataFromTree by calling renderToStaticMarkup repeatedly, collecting any Query data promises and awaiting them between renderings. The number of renderings should never be greater than the length of the longest chain of nested Query components (plus one), and the result of the final rendering is a Promise<string> whose HTML value can be used immediately, instead of calling renderToString again.

This approach is worse in some ways than our previous approach, since parts of the tree get re-rendered multiple times, and HTML is generated unnecessarily. Also, exceptions thrown during component rendering that are not related to data fetching can now abort the entire rendering, whereas previously we trapped errors within their specific subtrees. This was possible because we controlled the entire "rendering" process. Now that we're putting our faith in ReactDOM, we just have to roll with the punches, as far as exceptions are concerned. But that seems acceptable in practice.

On the upside, we have a fully hooks-compatible implementation of getDataFromTree on the same day hooks were announced!

benjamn · 2018-10-26T01:24:37Z

The bundlesize test is failing because the react-dom/server is now included in the bundle that's being measured, even though it's an npm package that can be shared with other packages.

benjamn · 2018-10-26T01:26:03Z

src/browser.ts

@@ -23,6 +23,8 @@ export * from './withApollo';

 export * from './types';

+export * from './walkTree';


Can we just avoid exporting this? Does anyone actually use walkTree directly (not through getDataFromTree), besides the tests in this repository?

I have seen some people using it, but not many. I did a quick GH code search, and can find it being referenced in a couple of cases, but again not many. Maybe we consider dropping the export when we hit 3.0?

benjamn · 2018-10-26T01:27:06Z

test/client/getDataFromTree.test.tsx

+            thing: state.thing + 1,
+            userId: props.id,
+            client: apolloClient,
+          } as any));


Since we no longer control the way setState updater functions are called, we can't rely on the context being passed as a third parameter.

benjamn · 2018-10-26T01:28:23Z

test/client/getDataFromTree.test.tsx

+        throw new Error('Should have thrown an error');
+      }, e => {
+        expect(e.toString()).toEqual('Error: foo');
+        expect(e).toBe(fooError);


This test was previously enforcing the functionality where we tried to trap multiple exceptions within their subtrees. Needless to say, this test is less useful now that only one exception will be thrown.

benjamn · 2018-10-26T01:29:40Z

test/client/getDataFromTree.test.tsx

        const markup = ReactDOM.renderToString(app);
+        expect(markup).toEqual(html);


If you want to use ReactDOM.renderToString instead of renderToStaticMarkup, you can. The benefit here is that you can just call getDataFromTree once, without having to call renderToString afterwards.

hwillson

This all looks really great @benjamn! I'm curious about the performance implications, but definitely agree with this future proofing approach. Thanks!

hwillson · 2018-10-26T09:57:14Z

src/browser.ts

@@ -23,6 +23,8 @@ export * from './withApollo';

 export * from './types';

+export * from './walkTree';


I have seen some people using it, but not many. I did a quick GH code search, and can find it being referenced in a couple of cases, but again not many. Maybe we consider dropping the export when we hit 3.0?

peggyrayzis

This PR is amazing!! 😍I'm so happy you were able to get this working so quickly.

A couple questions:

Can we quantify the performance hit? It might be worth having Apollo SSR users with large apps (@staylor 👀) test this out so we know if it's viable in production.

Also, exceptions thrown during component rendering that are not related to data fetching can now abort the entire rendering, whereas previously we trapped errors within their specific subtrees.

This is consistent with how React handles errors that are uncaught by an error boundary, so I don't think it's a big deal. Do we know if this approach works with error boundaries? I'm assuming it does because rendering is performed by React, but it would be helpful to confirm so we can recommend it in the docs as a workaround.

benjamn · 2018-10-26T18:21:35Z

For the examples/ssr application, here's how I instrumented the SSR code:

const start = +new Date;
await getDataFromTree(WrappedApp);
const body = renderToString(WrappedApp);
console.log("server rendering took", new Date - start, "ms");
sink.renderIntoElementById('app', body);

Here are the timings for ten requests using the previous tree-walking implementation of getDataFromTree:

~/dev/react-apollo/examples/ssr% meteor
[[[[[ ~/dev/react-apollo/examples/ssr ]]]]]

=> Started proxy.
=> Started MongoDB.
=> Started your app.

=> App running at: http://localhost:3000/
I20181026-14:17:07.631(-4)? server rendering took 146 ms
I20181026-14:17:13.668(-4)? server rendering took 23 ms
I20181026-14:17:14.432(-4)? server rendering took 20 ms
I20181026-14:17:15.321(-4)? server rendering took 21 ms
I20181026-14:17:16.261(-4)? server rendering took 18 ms
I20181026-14:17:17.130(-4)? server rendering took 29 ms
I20181026-14:17:18.036(-4)? server rendering took 23 ms
I20181026-14:17:18.982(-4)? server rendering took 17 ms
I20181026-14:17:19.902(-4)? server rendering took 17 ms
I20181026-14:17:20.807(-4)? server rendering took 17 ms

Here are the timings for ten requests using the new implementation of getDataFromTree:

I20181026-14:19:07.723(-4)? server rendering took 202 ms
I20181026-14:19:11.579(-4)? server rendering took 27 ms
I20181026-14:19:12.565(-4)? server rendering took 17 ms
I20181026-14:19:13.579(-4)? server rendering took 23 ms
I20181026-14:19:14.593(-4)? server rendering took 21 ms
I20181026-14:19:15.623(-4)? server rendering took 20 ms
I20181026-14:19:16.639(-4)? server rendering took 15 ms
I20181026-14:19:17.605(-4)? server rendering took 11 ms
I20181026-14:19:18.578(-4)? server rendering took 18 ms
I20181026-14:19:19.567(-4)? server rendering took 20 ms

If I only use renderToStaticMarkup, and just render the final result of getDataFromTree (instead of calling renderToString again), it gets a little better:

I20181026-14:19:52.365(-4)? server rendering took 80 ms
I20181026-14:19:53.485(-4)? server rendering took 15 ms
I20181026-14:19:54.436(-4)? server rendering took 16 ms
I20181026-14:19:55.293(-4)? server rendering took 17 ms
I20181026-14:19:56.162(-4)? server rendering took 19 ms
I20181026-14:19:57.004(-4)? server rendering took 26 ms
I20181026-14:19:57.790(-4)? server rendering took 15 ms
I20181026-14:19:58.636(-4)? server rendering took 15 ms
I20181026-14:19:59.481(-4)? server rendering took 20 ms
I20181026-14:20:00.210(-4)? server rendering took 15 ms

In other words, there's no apparent performance regression for small component trees. Here's the entire server-rendered HTML string for this app (to quantify "small"):

<div><div><div><h3>R2-D2</h3><h6>Luke Skywalker: newhope, empire, jedi</h6><h6>Han Solo: newhope, empire, jedi</h6><h6>Leia Organa: newhope, empire, jedi</h6></div></div></div>

My hunch is that these rendering times are dominated by the fetchData promises, not the rendering itself, but that's not surprising since the component tree is so small.

benjamn · 2018-10-26T18:26:39Z

@peggyrayzis Unfortunately, it sounds like error boundaries (specifically componentDidCatch) are not supported during SSR (in React 16, at least): facebook/react#10442. So this is probably another thing that will have to wait for first-class async server rendering support.

benjamn · 2018-10-26T18:41:52Z

Data confirming that fetchData (which communicates with the local GraphQL server) takes roughly 80%+ the time here (this is with the new getDataFromTree implementation):

91 'ms to resolve' { data: 
   { hero: 
      { name: 'R2-D2',
        id: '2001',
        friends: [Array],
        __typename: 'Droid' } },
  loading: false,
  networkStatus: 7,
  stale: false }
server rendering took 112 ms

27 'ms to resolve' { ... } 
server rendering took 33 ms

25 'ms to resolve' { ... } 
server rendering took 29 ms

19 'ms to resolve' { ... } 
server rendering took 20 ms

21 'ms to resolve' { ... } 
server rendering took 22 ms

28 'ms to resolve' { ... } 
server rendering took 32 ms

24 'ms to resolve' { ... } 
server rendering took 26 ms

28 'ms to resolve' { ... } 
server rendering took 30 ms

21 'ms to resolve' { ... } 
server rendering took 24 ms

23 'ms to resolve' { ... } 
server rendering took 26 ms

19 'ms to resolve' { ... } 
server rendering took 22 ms

Here's how I instrumented the code:

if (!this.queryGraveyard.has(query, JSON.stringify(variables))) {
  const start = Date.now();
  this.queryPromises.set(
    queryInstance,
    new Promise(resolve => {
      resolve(queryInstance.fetchData());
    }).then(result => {
      console.log(Date.now() - start, "ms to resolve", result);
      return result;
    })
  );
  ...

Note that every request gets a new ApolloClient and InMemoryCache instance, so the extra time taken by the first request is not because of caching. I guess there's just some additional initialization overhead for the first time calling renderToStaticMarkup, but it gets amortized once the server is up and running.

tizmagik · 2018-10-28T00:43:45Z

This is great, I’m especially glad to hear about moving off a parallel implementation of walking the tree and instead relying on the public React API.

However, I’m definitely concerned about the performance impact this would have, since renderToStaticMarkup() is synchronous, I expect a non-trivial impact on the performance for large apps (NYTimes). I can try and pull this branch down and run it against our app to see how it performs and post back, but in case it is severely impacted, do we have a plan B?

tizmagik · 2018-10-28T00:47:06Z

If you have any specific test procedures/parameters you’d like me to use to benchmark, please share and we’d be happy to run them. Otherwise will try and run some ab runs and compare the numbers.

Is this branch published/publishable anywhere to ease testing?

tizmagik · 2018-10-28T02:49:02Z

For now, this PR implements getDataFromTree by calling renderToStaticMarkup repeatedly, collecting any Query data promises and awaiting them between renderings. The number of renderings should never be greater than the length of the longest chain of nested Query components (plus one), and the result of the final rendering is a Promise<string> whose HTML value can be used immediately, instead of calling renderToString again.

@benjamn -- is it known when this last render is? If so, would it be possible to have this last render optionally do a renderToString() (instead of a renderToStaticMarkup())? This is because if an app is calling hydrate() on the client, the markup needs to be from renderToString(), so this would save an additional render in userland for some apps.

mike-marcacci · 2018-10-28T19:31:48Z

This is a very solid strategy, and is almost identical to how I implemented our SSR for Relay (before we moved over to Apollo). It's a far less brittle solution than walking the tree, and is worth whatever performance overhead it may add.

As I'm sure we all know, the react team is still plugging away on suspense and async rendering, which will eventually add first-class support for this kind of thing. In the meantime, I think the priority should be providing a way to handle SSR with Apollo which doesn't break on each new React release. Using renderToStaticMarkup to populate the store is a really great way to do this.

tmeasday · 2018-10-29T01:17:40Z

@benjamn keep in mind that it isn't just rendering latency that is the problem; even if unnecessary CPU load (rendering strings) is only contributing a small percentage to the latency, it can seriously affect throughput by multiplying per-request CPU usage significantly.

Having said that, I think it's the right choice!

Plus I'm generally an advocate of not layering your GraphQL queries too deep anyway -- if you have only one or two queries per page this isn't going to be a big performance hit for you, right?

benjamn · 2018-10-29T16:51:35Z

Plus I'm generally an advocate of not layering your GraphQL queries too deep anyway -- if you have only one or two queries per page this isn't going to be a big performance hit for you, right?

@tmeasday That's right! If you don't have any Query components that are descendants of other Query components, then this approach will only call the rendering function twice (once to collect the single layer of Query components, and once to render with the cached data). And if you use renderToString instead of renderToStaticMarkup, and reuse the final HTML string instead of calling renderToString again, then you'll be doing about the same amount of work as before:

// This HTML should be renderable as-is:
const html = await getDataFromTree(root, renderToString);
// No need to call renderToString(root) here!

benjamn · 2018-10-29T16:56:59Z

In case anyone has been itching to try this out, I just published [email protected] with these changes included.

benjamn · 2018-10-29T17:07:25Z

is it known when this last render is? If so, would it be possible to have this last render optionally do a renderToString() (instead of a renderToStaticMarkup())?

@tizmagik I see your point, but unfortunately I don't think it's possible to detect when the last rendering is going to happen, since it's now up to ReactDOM to traverse the component tree to find any remaining Query components. Even the first rendering could be the last, if no Query components are found.

I think the benefit of getting your final HTML from await getDataFromTree(root, renderToString) should outweigh the additional cost of using renderToString for rendering, especially if you make an effort to flatten your Query component hierarchy. In other words, if calling renderToString twice is really more expensive than calling renderToStaticMarkup twice followed by calling renderToString once, then renderToString must be more than twice as slow as renderToStaticMarkup, which would be surprising.

tizmagik · 2018-10-31T18:30:18Z

@benjamn instead of rendering to string, would it make sense to try using react-reconciler to basically write a renderToMemory() renderer to avoid the CPU-blocking step of producing the HTML markup that's thrown away on each successive render down the tree?

Admittedly, such an implementation would be more effort than this PR, but should hopefully be easier to maintain/support compared to the custom walker that's in master, as the heavy lifting/implementation details are mostly left up to react-reconciler. Thoughts?

(I'm still working on getting some real-world numbers from using this in our app and will post back)

JoviDeCroock

This also fixes what I managed to fix here: #2558

tizmagik · 2018-11-01T15:04:03Z

Thanks @benjamn -- agreed this is a good approach for now, and the fact that the renderer is configurable means one could implement a sort of renderToMemory() using react-reconciler instead of the renderToString() approach.

I might look into doing just that, would you be open to a PR (for documenting the option, if anything)?

benjamn · 2018-11-02T02:04:47Z

@tizmagik I think I would like to see react-reconciler maintained a little more actively with respect to development of react and react-dom. Right now it seems to lag behind the other packages. That said, it would be great to see how much of a difference it makes!

examples/ssr/server/main.js

@evenchange4

In [email protected], the rootContext parameter was replaced by the custom renderFunction parameter, which was a breaking change: #2533 (comment) This commit restores the previous API, while providing an alternative API that is more flexible and more accurately named: getMarkupFromTree. If you want to specify a custom rendering function, you should now use getMarkupFromTree instead of getDataFromTree: import { getMarkupFromTree } from "react-apollo"; import { renderToString } from "react-dom/server"; getMarkupFromTree({ tree: <App/>, // optional; defaults to {} context: { ... }, // optional; defaults to renderToStaticMarkup renderFunction: renderToString, }).then(markup => { // Use the markup returned by the final rendering... }); Hopefully, since [email protected] was never assigned the `latest` tag on npm, we can push this change without another minor version bump. cc @evenchange4 @hwillson

@evenchange4

In [email protected], the rootContext parameter was replaced by the custom renderFunction parameter, which was a breaking change: #2533 (comment) This commit restores the previous API, while providing an alternative API that is more flexible and more accurately named: getMarkupFromTree. If you want to specify a custom rendering function, you should now use getMarkupFromTree instead of getDataFromTree: import { getMarkupFromTree } from "react-apollo"; import { renderToString } from "react-dom/server"; getMarkupFromTree({ tree: <App/>, // optional; defaults to {} context: { ... }, // optional; defaults to renderToStaticMarkup renderFunction: renderToString, }).then(markup => { // Use the markup returned by the final rendering... }); Hopefully, since [email protected] was never assigned the `latest` tag on npm, we can push this change without another minor version bump. cc @evenchange4 @hwillson

klarstrup · 2018-11-22T03:38:21Z

For anyone else having piggybacked on old getDataFromTree's fetchData behavior for non-apollo things(I was using it to await an async Redux thunk), it's because this PR and thus 2.3.0+ removed it.

I know this was probably never intended to be used like that, but uhhhhh a lil note in the changelog would've saved me half a day of debugging. 😅

OllieJennings · 2019-01-24T16:21:48Z

@benjamn I know this has been merged, but can we please get some support with this issue (re-opening it could be a start): #615 its been around for almost 2yrs with nobody looking into it.

I know the Apollo guys have a lot of things going on with new features etc, but i just don't think SSR should be forgotten, and solving this issue will give Apollo the whole package, making it seamless to add SSR to an Apollo app, since all errors would be handled the exact same way.

hwillson · 2019-01-24T17:33:34Z

@OllieJennings I've re-opened #615. Thanks for the heads up!

(and quick FYI - you @ mentioned the wrong Ben - easy to do since benhjames often comes up first in the autocomplete list, but you wanted @benjamn)

OllieJennings · 2019-01-24T18:11:02Z

@hwillson cheers, yeah they look so similar, my bad there.

When we stopped reexporting everything from walkTree.ts as part of #2533, we also stopped exporting the Context type that it uses. Moving the declaration and export of Context from walkTree.ts to types.ts restores the ability to do import { Context } from "react-apollo" which should fix #2824.

`walkTree` is no longer used by `react-apollo` (see #2533), but since it was previously exported and made available externally, it is still available (albeit deprecated) until `react-apollo` 3.0. Unfortunately, the current `walkTree` implementation does not fully work with React's new Context API. It can be updated to work with modern versions of React, but the entire point of #2533 is to avoid spending anymore time working on `walkTree`, since its implementation relies on React internals (and is very brittle). Since external use of `walkTree` is very minimal, and since there have been no complaints of `walkTree` not working with React's new Context API since it has been made a standalone module (these problems were only noticed when `walkTree` was being used by `react-apollo`), I think we should leave `walkTree` as is. This commit makes sure `walkTree` passes Typescript compilation, but disables failing tests caused by render tree differences in newer versions of React.

benjamn requested review from hwillson, jbaxleyiii and peggyrayzis October 26, 2018 01:21

benjamn commented Oct 26, 2018

View reviewed changes

hwillson approved these changes Oct 26, 2018

View reviewed changes

Akryum mentioned this pull request Oct 26, 2018

[SSR] Fake instances tree for smart prefetching vuejs/core#7

Closed

peggyrayzis approved these changes Oct 26, 2018

View reviewed changes

benjamn added 3 commits October 29, 2018 12:31

Reimplement getDataFromTree in terms of renderToStaticMarkup.

4726c6c

Update examples/ssr app to Meteor 1.8.

2ac70aa

Bump react-apollo to version 2.3.0-beta.0.

347b7a8

benjamn force-pushed the reimplement-getDataFromTree-using-renderToStaticMarkup branch from 12222d6 to 347b7a8 Compare October 29, 2018 16:36

Mention getDataFromTree changes in Changelog.md.

76f9a43

jaydenseric mentioned this pull request Nov 1, 2018

Deprecate context object as a consumer and add a warning message facebook/react#13829

Merged

JoviDeCroock approved these changes Nov 1, 2018

View reviewed changes

benjamn deleted the reimplement-getDataFromTree-using-renderToStaticMarkup branch November 1, 2018 22:19

trojanowski mentioned this pull request Nov 5, 2018

Suspense and server side rendering trojanowski/react-apollo-hooks#8

Closed

0xycvv mentioned this pull request Nov 6, 2018

NextJS example won't Server Side Render after combining "with-apollo" example i18next/react-i18next#593

Closed

lfades mentioned this pull request Nov 8, 2018

React Hooks support lfades/next-with-apollo#31

Closed

steelbrain reviewed Nov 12, 2018

View reviewed changes

examples/ssr/server/main.js Show resolved Hide resolved

benjamn mentioned this pull request Nov 12, 2018

Restore getDataFromTree(rootElement, rootContext) API. #2586

Merged

dependencies bot mentioned this pull request Nov 15, 2018

Update react-apollo in / from 2.0.1 to 2.3.1 LibreTubeApp/LibreTube#254

Open

iquabius mentioned this pull request Nov 20, 2018

forwardRef prevent data to be load server side in child component apollographql/apollo-client#3955

Closed

isBatak mentioned this pull request Jan 9, 2019

Proof of concept React HOC components for datx-jsonapi infinum/datx#94

Closed

benjamn mentioned this pull request Mar 1, 2019

Export Context type from types.ts instead of walkTree.ts. #2832

Merged

smably mentioned this pull request Mar 11, 2019

Bundle size increase from =>2.3.0: react-dom-server.browser.production added. Why? #2828

Closed

hwillson mentioned this pull request Mar 16, 2019

Full dep/devDep updates to latest versions #2873

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reimplement getDataFromTree using ReactDOM.renderToStaticMarkup. #2533

Reimplement getDataFromTree using ReactDOM.renderToStaticMarkup. #2533

benjamn commented Oct 26, 2018 •

edited

Loading

benjamn commented Oct 26, 2018

benjamn Oct 26, 2018

hwillson Oct 26, 2018

benjamn Oct 26, 2018

benjamn Oct 26, 2018

benjamn Oct 26, 2018

hwillson left a comment

hwillson Oct 26, 2018

peggyrayzis left a comment

benjamn commented Oct 26, 2018

benjamn commented Oct 26, 2018 •

edited

Loading

benjamn commented Oct 26, 2018 •

edited

Loading

tizmagik commented Oct 28, 2018 •

edited

Loading

tizmagik commented Oct 28, 2018 •

edited

Loading

tizmagik commented Oct 28, 2018

mike-marcacci commented Oct 28, 2018 •

edited

Loading

tmeasday commented Oct 29, 2018

benjamn commented Oct 29, 2018 •

edited

Loading

benjamn commented Oct 29, 2018

benjamn commented Oct 29, 2018

tizmagik commented Oct 31, 2018 •

edited

Loading

JoviDeCroock left a comment •

edited

Loading

tizmagik commented Nov 1, 2018 •

edited by benjamn

Loading

benjamn commented Nov 2, 2018

klarstrup commented Nov 22, 2018

OllieJennings commented Jan 24, 2019 •

edited by benjamn

Loading

hwillson commented Jan 24, 2019

OllieJennings commented Jan 24, 2019

		@@ -23,6 +23,8 @@ export * from './withApollo';

		export * from './types';

		export * from './walkTree';

		const markup = ReactDOM.renderToString(app);
		expect(markup).toEqual(html);

Reimplement getDataFromTree using ReactDOM.renderToStaticMarkup. #2533

Reimplement getDataFromTree using ReactDOM.renderToStaticMarkup. #2533

Conversation

benjamn commented Oct 26, 2018 • edited Loading

benjamn commented Oct 26, 2018

benjamn Oct 26, 2018

Choose a reason for hiding this comment

hwillson Oct 26, 2018

Choose a reason for hiding this comment

benjamn Oct 26, 2018

Choose a reason for hiding this comment

benjamn Oct 26, 2018

Choose a reason for hiding this comment

benjamn Oct 26, 2018

Choose a reason for hiding this comment

hwillson left a comment

Choose a reason for hiding this comment

hwillson Oct 26, 2018

Choose a reason for hiding this comment

peggyrayzis left a comment

Choose a reason for hiding this comment

benjamn commented Oct 26, 2018

benjamn commented Oct 26, 2018 • edited Loading

benjamn commented Oct 26, 2018 • edited Loading

tizmagik commented Oct 28, 2018 • edited Loading

tizmagik commented Oct 28, 2018 • edited Loading

tizmagik commented Oct 28, 2018

mike-marcacci commented Oct 28, 2018 • edited Loading

tmeasday commented Oct 29, 2018

benjamn commented Oct 29, 2018 • edited Loading

benjamn commented Oct 29, 2018

benjamn commented Oct 29, 2018

tizmagik commented Oct 31, 2018 • edited Loading

JoviDeCroock left a comment • edited Loading

Choose a reason for hiding this comment

tizmagik commented Nov 1, 2018 • edited by benjamn Loading

benjamn commented Nov 2, 2018

klarstrup commented Nov 22, 2018

OllieJennings commented Jan 24, 2019 • edited by benjamn Loading

hwillson commented Jan 24, 2019

OllieJennings commented Jan 24, 2019

benjamn commented Oct 26, 2018 •

edited

Loading

benjamn commented Oct 26, 2018 •

edited

Loading

benjamn commented Oct 26, 2018 •

edited

Loading

tizmagik commented Oct 28, 2018 •

edited

Loading

tizmagik commented Oct 28, 2018 •

edited

Loading

mike-marcacci commented Oct 28, 2018 •

edited

Loading

benjamn commented Oct 29, 2018 •

edited

Loading

tizmagik commented Oct 31, 2018 •

edited

Loading

JoviDeCroock left a comment •

edited

Loading

tizmagik commented Nov 1, 2018 •

edited by benjamn

Loading

OllieJennings commented Jan 24, 2019 •

edited by benjamn

Loading