Support tracked and other improvements #1035

patricklx · 2019-09-22T07:25:10Z

Dont use observe for update properties
Add icons for tracked, function, getter
support computed that return undefined
sort and group properties by type.
Service -> Tracked -> Computed -> Getter -> Property -> Function
support complex getters, can always be calculcated when needed
include functions, (why not?)

RobbieTheWagner · 2019-09-22T11:27:07Z

Hi @patricklx, thanks so much for the PR! Supporting tracked would be 💯

RobbieTheWagner · 2019-09-22T11:44:05Z

@patricklx I apologize, but I just merged a PR that moved around some things, so there are some conflicts you will need to resolve. Sorry about that!

nummi · 2019-09-23T21:18:36Z

@patricklx, is this still a work in progress? When I try to run this locally I get this error: Uncaught TypeError: desc.hasOwnProperty is not a function

patricklx · 2019-09-24T07:12:52Z

@nummi mmm, its working fine for me...
when does it happen? Do you select a specific object, component, service, mixin... ?

I have a project where I still have both types, EmberObject.extend or es6 class.
It's working everywhere for routes, controller, components and services...

patricklx · 2019-09-24T10:09:39Z

Working on fixing the tests...

nummi · 2019-09-24T17:03:41Z

EDIT: FIXED 🎉

https://deprecations.emberjs.com

VM77:3331 Uncaught TypeError: desc.hasOwnProperty is not a function

patricklx · 2019-09-25T16:01:42Z

@rwwagner90 @nummi this should be quite ready now. Many changes...
I hope you agree with them :)

nummi · 2019-09-25T16:39:04Z

app/styles/object_inspector.scss

+  background-color: var(--base08);
+
+  &:before {
+    content: "F";


How about "ƒ" ? :)

RobbieTheWagner · 2019-09-25T17:26:47Z

Thanks so much for this PR @patricklx! This is super amazing and we really appreciate your help! We will review this in the coming days and get it in ASAP 😃

RobbieTheWagner · 2019-09-26T14:18:22Z

@patricklx please let us know when this is done and ready for review!

patricklx · 2019-09-26T14:25:37Z

@rwwagner90 it's ready now :)

RobbieTheWagner · 2019-09-26T17:10:46Z

@patricklx it looks like we have a couple test failures. I know you already fixed a lot of failures, but do you mind taking a look?

nummi · 2019-09-26T18:59:36Z

app/controllers/application.js

@@ -78,6 +78,9 @@ export default Controller.extend({
   * Called when inspecting an object from outside of the ObjectInspector
  */
  activateMixinDetails(name, objectId, details, errors) {
+    if (this.mixinStack.objectAt(0) && this.mixinStack.objectAt(0).objectId === objectId) {


What is this check preventing?

If there are many renders, it will remove and add the same object in the inspector everytime, therefore making it impossible to go into properties and it also decreases the performance of the inspector. (If by some mistake you have 100ths of renders / s ...)

If you end up adding another commit to this PR would you mind putting in a comment?

Would it perhaps be a good idea to wrap in some sort of throttle or debounce? That way if we render hundreds of times, we could make it only actually render a few of those times.

actually, I remember the real issue now.
When a rerender happens, and i'm currently digging into an object, it will reset the object inspector. Throwing away my current inspection.
I will update the comment

I had many, yes. fixes it, but still once every few seconds, and it will reset the object inspector...

its only gets outdated if new properties are added after the object was created. But if you switch between objects in the inspector it will refresh

What is your app doing that is making it constantly refresh? I think that sounds like an issue with the app, not inspector.

If we want to not refresh, we should check for deep equality of the properties in the object you are inspecting, if nothing changes, there is no need to refresh, but if it does we need to refresh or you are looking at the wrong info.

it is an issue with the app, but i think we should be able to use ember-inspector during app development with app issues :)

but I just tested without this change, and its working fine now...

I think we either should remove this check entirely or it should be a check that basically nothing on the instance changed. If all the properties remain the same, skip refreshing, etc. Might be easier to just remove for now.

RobbieTheWagner · 2019-09-27T11:57:20Z

@pzuraq wanted to get your opinions on these changes, as they are around tracked and I don't know much about that yet.

patricklx · 2019-09-27T12:02:48Z

parts for tracked:
https://github.com/emberjs/ember-inspector/pull/1035/files#diff-7c2c7ea1dfc0ef384d1ec6e685711d15R163
and
https://github.com/emberjs/ember-inspector/pull/1035/files#diff-7c2c7ea1dfc0ef384d1ec6e685711d15R720

RobbieTheWagner · 2019-09-27T13:55:58Z

@patricklx have you been testing this against an app? If so, is it something we could also use for testing?

patricklx · 2019-10-03T07:59:38Z

Hi, @rwwagner90
Yes, I've been testing with an app.
Got the dev build up:
http://116.203.88.77:9020
Login: test, test

patricklx · 2019-10-07T10:58:54Z

seems it will take some time to review the tracked part?

otherwise, here are some hints for tracked if you want to review yourself:
look at computed in emberjs:
https://github.com/emberjs/ember.js/blob/016f8066d0b484f3f69ce8c337be3b8b8aa5fce4/packages/%40ember/-internals/metal/lib/computed.ts#L545
https://github.com/emberjs/ember.js/blob/016f8066d0b484f3f69ce8c337be3b8b8aa5fce4/packages/%40ember/-internals/metal/lib/computed.ts#L528
https://github.com/emberjs/ember.js/blob/016f8066d0b484f3f69ce8c337be3b8b8aa5fce4/packages/%40ember/-internals/metal/lib/computed.ts#L532

also note that tagForProperty will setup mandatory setters, which we want to prevent while using ember-inspector
https://github.com/emberjs/ember.js/blob/master/packages/%40ember/-internals/metal/lib/tags.ts#L36
https://github.com/emberjs/ember.js/blob/master/packages/@ember/-internals/metal/lib/tracked.ts#L193

RobbieTheWagner · 2019-10-18T13:47:22Z

ember_debug/object-inspector.js

@@ -500,7 +500,7 @@ export default EmberObject.extend(PortMixin, {
      if (proto.hasOwnProperty('toString') && !proto.hasOwnProperty('hasOwnProperty')) {
        return proto.toString();
      }
-      if (name === 'Class' || name.startsWith('_')) {
+      if (name === 'Class' || name.startsWith('_') || name.length === 1) {


What is the use case for name.length === 1?

in prod build the class names are uglyfied to 1 character

@patricklx wouldn't that not always be the case though? What if you have more classes than the 26 letters in the alphabet? Would it make it 2 characters? @pzuraq @rwjblue is there a more reliable check we can do here?

mmm, right, it could happen. but I think it must be more than 26 classes in a module or in same scope?
But I guess we could set it to length <= 3? should be 17,576‬ different classes.

I would like to hear from @pzuraq, @chancancode, or @rwjblue on if there is something less brittle we could do for this.

I think it's more related to variables in scope, classes tend to be the top level variables of a module, but you could definitely have others, maybe more than 26. In any case, I do think this is pretty brittle.

My question is, what is this case handling that the others don't? I believe proto.toString() should cover anything resolved by the container, and class.constructor.name should cover most other things. I suppose if there's another toString() that is defined somewhere in the prototype chain that could be valuable - either someone manually put one there, or it's one of the default <unknown:mixin> type ones. In that case, maybe we can either:

Walk up the prototype chain to see if we have a toString() at all (that isn't the standard Object.toString()) or

Just go with the default class name. Even minified, it may be a bit more useful than a generic <unknown:mixin> (though it'd be less useful than a custom one, probably)

but if we only use prototype lookup, it would show to the toString from mixins...
And only default class name will not show the toString for mixins

From what I can see, toString from mixins should be applied to the class, and shouldn't belong to the mixin directly. I actually can't find a way to name mixins at all, which is a bit unfortunate, but that seems to be by design 😕

RobbieTheWagner · 2019-10-18T13:48:26Z

ember_debug/object-inspector.js

@@ -842,6 +845,12 @@ function calculateCPs(object, mixinDetails, errorsForObject, expensiveProperties
            tagInfo.tag = metal.track(() => {
              value = calculateCP(object, item.name, errorsForObject);
            });
+            if (!tagInfo.tag.subtag && !tagInfo.tag.subtags && tagInfo.tag === metal.tagForProperty(object, item.name)) {


I'm not sure what these ifs are for, but it seems like maybe it should be a util function to check for whatever this is checking?

this is to detect tracked in prod builds.
if only a tracked is in a tracking context metal.track(...), then the resulting tag will be the same as the metal.tagForProperty and will not have any subtag or subtags.
I think emberjs should provide a way to check if a property on an object (or descriptor) is a tracked. Then we can remove this in the feature

@chancancode @pzuraq thoughts on this?

I actually think this should be the only way we do this, the above code that detects CURRENT_TRACKER is much more brittle, it could break at any time. This is more solid, it likely won't break as long as we have Ember.meta.

We should get rid of the check for subtag and subtags and instead move the next if checks up, if the value has:

Is an accessor, e.g. has get in its descriptor

Has a tagForProperty

Is not a computed or service

Then it is a tracked property.

pzuraq · 2019-10-22T00:05:48Z

app/components/object-inspector/sort-properties.js

@@ -21,7 +46,15 @@ export default Component.extend({
     * @property sortProperties
     * @type {Array<String>}
     */
-    this.sortProperties = ['name'];
+    this.sortProperties = [
+      'isFunction',


I think special casing arrays would be fine, this could also be done in a followup though. I agree that properties and tracked fields should likely be grouped together, the eslint rule seems to make the most sense. I'd say let's do that to get this merged, then followup to improve the story for arrays specifically.

pzuraq · 2019-10-22T00:12:39Z

ember_debug/object-inspector.js

+
+function getTagTrackedProps(tag, ownTag, level=0) {
+  const props = [];
+  if (!tag || level > 1) {


We should add a comment here, if I'm thinking this through right the logic is because you don't want to track indirect dependencies (e.g. dependencies of CPs that get tracked, etc)

pzuraq · 2019-10-22T00:24:25Z

ember_debug/object-inspector.js

@@ -500,7 +500,7 @@ export default EmberObject.extend(PortMixin, {
      if (proto.hasOwnProperty('toString') && !proto.hasOwnProperty('hasOwnProperty')) {
        return proto.toString();
      }
-      if (name === 'Class' || name.startsWith('_')) {
+      if (name === 'Class' || name.startsWith('_') || name.length === 1) {


I think it's more related to variables in scope, classes tend to be the top level variables of a module, but you could definitely have others, maybe more than 26. In any case, I do think this is pretty brittle.

My question is, what is this case handling that the others don't? I believe proto.toString() should cover anything resolved by the container, and class.constructor.name should cover most other things. I suppose if there's another toString() that is defined somewhere in the prototype chain that could be valuable - either someone manually put one there, or it's one of the default <unknown:mixin> type ones. In that case, maybe we can either:

Walk up the prototype chain to see if we have a toString() at all (that isn't the standard Object.toString()) or

Just go with the default class name. Even minified, it may be a bit more useful than a generic <unknown:mixin> (though it'd be less useful than a custom one, probably)

pzuraq · 2019-10-22T00:29:00Z

ember_debug/object-inspector.js

+ * @param value
+ * @returns {string}
+ */
+function typeOf(value) {


This does seem like a bug in Ember, but it's probably fine for now to fix it here and try to upstream the fix later.

pzuraq · 2019-10-22T00:49:52Z

ember_debug/object-inspector.js

@@ -842,6 +845,12 @@ function calculateCPs(object, mixinDetails, errorsForObject, expensiveProperties
            tagInfo.tag = metal.track(() => {
              value = calculateCP(object, item.name, errorsForObject);
            });
+            if (!tagInfo.tag.subtag && !tagInfo.tag.subtags && tagInfo.tag === metal.tagForProperty(object, item.name)) {


I actually think this should be the only way we do this, the above code that detects CURRENT_TRACKER is much more brittle, it could break at any time. This is more solid, it likely won't break as long as we have Ember.meta.

We should get rid of the check for subtag and subtags and instead move the next if checks up, if the value has:

Is an accessor, e.g. has get in its descriptor

Has a tagForProperty

Is not a computed or service

Then it is a tracked property.

pzuraq · 2019-10-22T01:02:08Z

ember_debug/object-inspector.js

+      objectDebugInfo = null;
+    }
+    if (object.content) {
+      object = object.content;


I definitely think skipping over proxies is a bad idea. I know that 9 times out of 10, they're a pretty transparent wrapper, but they can have their own state and such (not a great design pattern, but it is what it is)

mmm, but for ember data it had issues with displaying data.
the _debugInfo is accessed with get, which passes through ember proxy.
It's also a bit annoying to click 2 times whenever I want to inspect e.g. belongsTo data...
Can we make a special case for ember data models?

So, loaded up an Ember app to take a look, emberclear, and it looks like currently at least models are already special cased. Whenever I click on a model, and then on a relationship within that model, it shows the model itself, not the proxy first. I think keeping that behavior would be fine since it's already established.

FWIW, I don't necessarily think this would be a bad idea either, but it seems out of scope for this PR. I think we should fix the existing behavior for sure, but any additional proxy changes should be followups.

strange, for me it keeps failing in ember-data _debugInfo... version 3.11.4, but sure, i can extract this parts into another PR

pzuraq · 2019-10-22T01:03:24Z

ember_debug/object-inspector.js

+      }
+
+      if (!options.isService) {
+        options.isService = desc._getter && desc._getter.name === 'getInjection';


This seems pretty brittle. I think I'd rather stick with the previous two checks, and drop this one for now.

pzuraq · 2019-10-22T01:19:47Z

ember_debug/object-inspector.js

    });

-    mixins.mixins.push(mix);
+    mixin.properties = propertiesForMixin({ mixins: [mixin] });


Suggested change

mixin.properties = propertiesForMixin({ mixins: [mixin] });

// Clean the properties, removing private props and bindings, etc

mixin.properties = addProperties(mixin, {});

pzuraq · 2019-10-22T02:17:31Z

ember_debug/object-inspector.js

    }
+    mixinDetails.push(...objectMixins.slice(0, index));
+    const objectName = mixinDetails[0].name;
+
    mixins.forEach(mixin => {


So, the new strategy appears to be:

Walk the entire prototype tree, gathering all information about the object/classes and putting it onto some fake-mixins

Put these on top, and then add mixins, and then add EmberObject at the very bottom

I think this is a bit problematic, because then the mixins are going to be out of order with the class prototypes they actually belong to. They'll also consist entirely of properties/descriptors that already show up on one of the prototype-mixins (unless they're overridden, which I guess technically they would all be?), e.g.:

class Foo extends Bar.extend(MyMixin) {} class Bar extends Baz {} class Baz extends EmberObject {}

// inspector shows - Foo - Bar - Baz - MyMixin - EmberObject

I think ideally what we should do here is show only the classes themselves by default, and then have a sort of subview where you can examine the mixins of a particular class:

// inspector shows - Foo - Bar - mixins: MyMixin - Baz - EmberObject

I think we could add the subview in a followup, but I'm not sure if we can show the out of order view in the meantime, that seems like it'd be really confusing. Maybe we could try to setup the order the correct way at least?

// inspector shows - Foo - Bar - MyMixin - Baz - EmberObject

I changed to logic for this part and added a test to check the correct order of prototypes and mixins

pzuraq · 2019-10-22T02:21:23Z

tests/ember_debug/object-inspector-test.js

@@ -296,12 +301,14 @@ module('Ember Debug - Object Inspector', function(hooks) {

    inspected.set('name', 'Alex');

+    await new Promise(res => setTimeout(res, 400));


At the least abstract this to be a separate helper I think

pzuraq · 2019-10-22T02:24:14Z

Overall this is looking pretty good, there are just a couple things that need to change I think to be consistent. I should be able to work on this tomorrow morning (I think I have push access to the branch as a maintainer), if that works, would definitely like to get this merged asap 😄

pzuraq · 2019-10-23T04:43:11Z

@patricklx Made a few changes to clean some things up. It's really important that we show an accurate representation of the object's inheritance, so really everything in the prototype chain should accurately only show its own properties. We're definitely lacking a couple of the APIs that would make this ideal, but I found a couple of ways to hack around it for the time being.

I think with these changes this should be ready to merge, assuming tests pass 🎉

patricklx · 2019-10-23T07:17:23Z

mmm, okay.
But now I only see some strange properties in the Own Properties, which might be correct, but not useful, since those are just set my ember itself, e.g. renderer and parentView etc...

Now, I always can close the Own Properties tab and open some weird named tab :) .
This worked with Ember.Object.extend, since it would set CPs and properties on the instance itself, but its not happening with es6 classes anymore, so Own Properties is not so useful for debugging/development. Can we have a Useful Info tab then.... where we merge Own Properties and the one from the instance class prototype?
In a previous merged PR I had the class name for classes for components/routes in a tab and made it expanded by default. Lets revert to that then?

And now the names of the my components are always <@ember/component:ember123> instead of showing the Classname, and if I didn't set a class name with export default class extends,
it shows me <_class2:ember123>

in the top of the object inspector it shows the mixin name (from toString) if there is one...

I cannot dig into any belongsTo in the models...
and I cannot get the content for PromiseManyArray, it just gets me the list of internal models when I click on .content. Both were fixed by including the proxy content. I would rather skip over the proxies, unless there is a special property set on them. e.g. _showProxyDetails
But I open another PR for that.

patricklx · 2019-10-23T09:42:39Z

@pzuraq I added one more commit to show the first class protoype be default, since it has all the properties. This is how it was before this PR

RobbieTheWagner · 2019-10-23T13:28:58Z

@pzuraq @patricklx we should try to preserve how the object inspector worked before. If it merged own properties and prototype properties, perhaps we should leave that?

patricklx · 2019-10-23T13:33:19Z

@rwwagner90 before it was showing own properties and the first class prototype separately. It was also expanding both.
I wanted to merge those, since those 2 have all the useful information. Now its back to 2 separated and expanded

RobbieTheWagner · 2019-10-23T13:35:36Z

@patricklx we should keep it how it was before 👍

RobbieTheWagner · 2019-10-23T13:42:31Z

@patricklx I know you've already put a lot of work in on this, but for some of the things you mentioned like the class names being different, not being able to expand belongsTo, proxies, etc it would be 💯 if we added some tests for some of these things, so we can ensure it keeps working.

pzuraq · 2019-10-23T14:20:24Z

I cannot dig into any belongsTo in the models...

This is not ideal, but this does appear to match the existing behavior. If I visit https://www.emberobserver.com/addons/ember-cli-babel and open the inspector, both before and after the change, relationships within the data store seem to work the same. We should definitely fix this, I just think it needs to be in a separate followup, with a lot of tests to make sure it doesn't get into this state again 😄

And now the names of the my components are always <@ember/component:ember123> instead of showing the Classname

Hmm, I was thinking when I rewrote it that this would be solved by us removing classic components and EmberObject as a whole, or updating the toString() behavior upstream. I also didn't want to remove the GUID stuff, in case some users were using it. However, it would be very nice to get some progressive enhancement here just by switching to native classes. Let me think about it for a minute..

Can we have a Useful Info tab then.... where we merge Own Properties and the one from the instance class prototype?

FWIW, I do think this would be useful. Like, normal JS objects in the console inspector do show all inherited values, but slightly greyed out. I think we can do something better here, but that's a somewhat orthogonal change to the main issues here, which are tracked support and getters, and better support for native classes.

It's generally a good idea to try to break large changes like these up into smaller PRs, so we can review them more thoroughly. I actually think there will still need to be quite a few refactors (as I noted in the comment I left) so there's lots of opportunity to get a better DX

pzuraq · 2019-10-23T14:49:26Z

Ok, updated it to be a bit more conservative, and to keep the GUID if it exists for the time being. In the long run I think we should be getting rid of that too, but for now I think this is a better intermediate step.

Thanks again @patricklx for all the work in this PR! Seriously, it's been incredibly helpful 😄 I definitely want to make sure we solve the issues you brought up around proxies as well, especially the ones surrounding Ember Data, it does seem like the object explorer is pretty limited around it at the moment. We have a lot more work to do here, so I think we can roll out incremental improvements as time goes on.

nummi · 2019-10-23T18:02:27Z

Thank you @patricklx 🎉

RobbieTheWagner · 2019-10-23T18:07:18Z

@patricklx we definitely owe you a 🍺for your help! Thanks so much!

patricklx · 2019-10-24T08:30:04Z

👍, thanks. And sorry that this PR got so big... 😅

* Support tracked and other improvements Dont use observe for update properties Add icons for tracked, function, getter support computed that return undefined group properties by type * add workaround for [object AsyncFunction] typeof just returns 'function' * improve production build detection of tracked and class names * fix * extract into helper * revert to previous inheritance/merging semantics * improve class name and inspector view * use classname only with ember toString * add more tests for new behavior * getter compat

patricklx force-pushed the support-refresh.same branch 2 times, most recently from 03ccee8 to dfa8764 Compare September 23, 2019 15:51

patricklx changed the title ~~[wip] Support tracked and other improvements~~ Support tracked and other improvements Sep 23, 2019

patricklx force-pushed the support-refresh.same branch from 705b8cb to 60c7268 Compare September 24, 2019 12:13

patricklx changed the title ~~Support tracked and other improvements~~ [wip] Support tracked and other improvements Sep 24, 2019

patricklx force-pushed the support-refresh.same branch from 985add0 to 8c98ed4 Compare September 25, 2019 15:30

patricklx changed the title ~~[wip] Support tracked and other improvements~~ Support tracked and other improvements Sep 25, 2019

patricklx force-pushed the support-refresh.same branch 2 times, most recently from 2892119 to 7fafb04 Compare September 25, 2019 16:17

nummi reviewed Sep 25, 2019

View reviewed changes

nummi reviewed Sep 26, 2019

View reviewed changes

RobbieTheWagner requested a review from pzuraq September 27, 2019 11:56

RobbieTheWagner requested review from wycats and krisselden October 7, 2019 17:18

improve production build detection of tracked and class names

ae32f42

RobbieTheWagner reviewed Oct 18, 2019

View reviewed changes

pzuraq suggested changes Oct 22, 2019

View reviewed changes

patricklx and others added 3 commits October 22, 2019 18:11

fix

0dbedc7

extract into helper

69b8107

revert to previous inheritance/merging semantics

08c31ec

improve class name and inspector view

5e1ce0c

use classname only with ember toString

57ff01f

Chris Garrett added 2 commits October 23, 2019 09:04

add more tests for new behavior

3f5300a

getter compat

b533e96

pzuraq approved these changes Oct 23, 2019

View reviewed changes

RobbieTheWagner approved these changes Oct 23, 2019

View reviewed changes

RobbieTheWagner merged commit e3ee13a into emberjs:master Oct 23, 2019

RobbieTheWagner added the enhancement label Oct 23, 2019

patricklx deleted the support-refresh.same branch October 24, 2019 08:37

chancancode mentioned this pull request Nov 9, 2019

Using Inspector (3.12.2) to view a particular Component or Data Model record fails with the following error: #1068

Closed

	mixin.properties = propertiesForMixin({ mixins: [mixin] });
	// Clean the properties, removing private props and bindings, etc
	mixin.properties = addProperties(mixin, {});

		@@ -296,12 +301,14 @@ module('Ember Debug - Object Inspector', function(hooks) {

		inspected.set('name', 'Alex');

		await new Promise(res => setTimeout(res, 400));

Support tracked and other improvements #1035

Support tracked and other improvements #1035

Conversation

patricklx commented Sep 22, 2019 • edited Loading

RobbieTheWagner commented Sep 22, 2019

RobbieTheWagner commented Sep 22, 2019

nummi commented Sep 23, 2019

patricklx commented Sep 24, 2019

patricklx commented Sep 24, 2019

nummi commented Sep 24, 2019 • edited Loading

EDIT: FIXED 🎉

patricklx commented Sep 25, 2019

Choose a reason for hiding this comment

RobbieTheWagner commented Sep 25, 2019

RobbieTheWagner commented Sep 26, 2019

patricklx commented Sep 26, 2019

RobbieTheWagner commented Sep 26, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RobbieTheWagner commented Sep 27, 2019

patricklx commented Sep 27, 2019

RobbieTheWagner commented Sep 27, 2019

patricklx commented Oct 3, 2019 • edited Loading

patricklx commented Oct 7, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patricklx Oct 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pzuraq commented Oct 22, 2019

pzuraq commented Oct 23, 2019

patricklx commented Oct 23, 2019 • edited Loading

patricklx commented Oct 23, 2019 • edited Loading

RobbieTheWagner commented Oct 23, 2019

patricklx commented Oct 23, 2019 • edited Loading

RobbieTheWagner commented Oct 23, 2019

RobbieTheWagner commented Oct 23, 2019

pzuraq commented Oct 23, 2019

pzuraq commented Oct 23, 2019

nummi commented Oct 23, 2019

RobbieTheWagner commented Oct 23, 2019

patricklx commented Oct 24, 2019

patricklx commented Sep 22, 2019 •

edited

Loading

nummi commented Sep 24, 2019 •

edited

Loading

patricklx commented Oct 3, 2019 •

edited

Loading

patricklx commented Oct 7, 2019 •

edited

Loading

patricklx Oct 18, 2019 •

edited

Loading

patricklx commented Oct 23, 2019 •

edited

Loading

patricklx commented Oct 23, 2019 •

edited

Loading

patricklx commented Oct 23, 2019 •

edited

Loading