Normative: Add initializer callback for side effects #165

littledan · 2018-10-30T15:40:15Z

Many decorators will want to perform a side effect when instantiating
a class, for example:

To select [[Set]] rather than [[DefineOwnProperty]] semantics for
defining a field
To store the field in an entirely different place, neither an ordinary
property or private field (e.g., MobX stores some fields in a Map)
To register the instance under construction in some way (e.g., from
a class decorator). "Instance finishers" have been proposed for this
purpose, but it's not clear when to run such a finisher. For many use
cases (e.g., interacting with the DOM), an instance "starter" is just
as good, due to run-to-completion semantics.

This patch adds a third decorator descriptor kind of "initializer" to register
an initializer to run, interspersed with field initializers.

This PR depends on #163 and #164

Fixes #44

littledan · 2018-10-30T15:41:23Z

cc @nicolo-ribaudo @wycats @mweststrate

ljharb

If key is null and any placement is provided, that looks to me like programmer error. Could we instead throw when a placement is provided unnecessarily, just like we throw in #30?

littledan · 2018-10-30T16:09:26Z

@ljharb I agree that error checking in general should be strict where there's no reasonable meaning, but in #57 I think we settled on not artificially restricting expressiveness where there is something that could make sense. That's the principle that I'm trying to follow here.

ljharb · 2018-10-30T16:16:15Z

How does providing a placement for “no property” make sense?

littledan · 2018-10-30T16:35:47Z

@ljharb It affects when it runs and what the receiver is. The typical usage will be to pass "own", so it's called on instances during instance construction. Not providing a placement is an error in this patch.

rdking · 2018-10-30T16:37:11Z

Isn't this trying entirely too hard to avoid the constructor? This is very quickly approaching the "magic" annotations of Java. Adding repeatable nonsense to the list of properties just to trigger side effects can:

make tooling more difficult to write since it would have to understand null as a "special" property name where currently {[null]:1} is perfectly valid (albeit weird) code.
confuse developers since multiple properties of the same name appear to be being defined on the object
be easier done and easier to understand just by using the tried and true constructor.

ljharb · 2018-10-30T16:41:13Z

@littledan gotcha, that makes sense then - thanks.

littledan · 2018-10-30T17:00:35Z

@rdking

For the {[null]:1} case: Note that the system will have already turned it into "null" by the time it reaches the decorator, so no loss in expressivity.
This patch doesn't affect that property; you can have duplicate (ordinary) field and method declarations without this patch (but it's not really useful, given how coalescing works)
The purpose here is to help frameworks or similar things insert logic into the path. Lots of people have run into similar problems, which led to decorators in general, and in particular this PR.

jridgewell · 2018-10-30T17:19:50Z

I like the idea, but it seems a bit strange to have a field name provided to the decorator for it to just throw it away. How hard would it be to have a decorator without a name, just a init?

class Ex {
  @dec x = 1
  // no init
  @sideEffect ;

  // or some init
  @sideEffect this.x = 2; // Option A
  @sideEffect { this.x = 2 }; // Option B
}

rdking · 2018-10-30T17:22:34Z

@jridgewell That makes more sense visually, but what would the decorator receive? Is the side effect independent of the field it's supposed to act against?

rdking · 2018-10-30T17:28:32Z

Better question, if the decorator is just supposed to trigger some side-effect without regard to any particular field, then why isn't it a class-level decorator?

littledan · 2018-10-30T18:35:49Z

@jridgewell I am not sure if we need that syntax extension. The hope here is that these side effects will be used by semantically field-like declarations, or added by a class decorator. Do you have a use case in mind for those other forms?

jridgewell · 2018-10-30T19:01:34Z

I guess not, it was just my first reaction.

littledan · 2018-10-30T19:18:44Z

I probably didn't explain the use cases very clearly in the above commit message. As an example, you could make a decorator to give a field declaration [[Set]] semantics, so the following would work:

class MyComponent extends HTMLElement {
  @set onclick = (event) => { /* ... */ }
}

As the following:

function set(desc) {
  assert(desc.placement === "own");
  assert(desc.kind === "field");
  assert((typeof desc.key)[0] === "s");  // symbol or string
  assert(desc.descriptor.configurable);
  assert(desc.descriptor.writable);
  assert(desc.descriptor.enumerable);
  return {key: null, placement: "own", descriptor: { },
          initializer() { this[desc.key] = desc.initializer.call(this) } };
}

nicolo-ribaudo · 2018-10-30T20:40:02Z

Instead of key: null, I think that kind: "initializer" would be better. An "initializer" must have the placement and initializer fields, and it throws if it has key or descriptor. Reusing existing kind: "field" to declare something that is not a field (since it doesn't get defined anywhere) seems like unnecessary overloading.

littledan · 2018-10-30T22:07:02Z

@nicolo-ribaudo That suggestion sounds totally reasonable to me. Want to write it up in a replacement PR?

nicolo-ribaudo · 2018-10-30T22:18:24Z

I can't probably do it this week since I don't have my PC, but if no one does it first I hope I will be able to do it next week.

Many decorators will want to perform a side effect when instantiating a class, for example: - To select [[Set]] rather than [[DefineOwnProperty]] semantics for defining a field - To store the field in an entirely different place, neither an ordinary property or private field (e.g., MobX stores some fields in a Map) - To register the instance under construction in some way (e.g., from a class decorator). "Instance finishers" have been proposed for this purpose, but it's not clear when to run such a finisher. For many use cases (e.g., interacting with the DOM), an instance "starter" is just as good, due to run-to-completion semantics. This patch permits decorators to create elements of the form { kind: "initializer", placement: "own", initializer: fn } to be used purely for their side effect, with evaluation semantics and ordering identical to fields, and interspersed with them in evaluation order.

littledan · 2018-11-02T16:11:45Z

Updated to include kind: "initializer" as recommended by @mweststrate and @nicolo-ribaudo . Documentation in the explainer still needed (anyone interested?). Reviews welcome!

nicolo-ribaudo

I don't remember its name right now, but there is a function which checks that there aren't duplicated keys which probably should be updated to handle ElementDescriptors without [[Key]].
Maybe we should throw if the initializer property of a { kind: "initializer" } element is undefined, since it would be almost certainly a programming error?

spec.html

nicolo-ribaudo · 2018-11-02T16:41:39Z

spec.html

@@ -689,7 +720,7 @@ <h1>DecorateConstructor ( _elements_, _decorators_ )</h1>
            1. Append _elementsAndFinisher_.[[Finisher]] to _finishers_.
          1. If _elementsAndFinisher_.[[Elements]] is not *undefined*,
            1. Set _elements_ to the concatenation of _elementsAndFinisher_.[[Elements]] and _privateElements_.
-            1. If there are two class elements _a_ and _b_ in _elements_ such that _a_.[[Key]] is _b_.[[Key]] and _a_.[[Placement]] is _b_.[[Placement]], throw a *TypeError* exception.
+            1. If there are two class elements _a_ and _b_ in _elements_ such that _a_.[[Kind]] is not `"initializer"` and _b_.[[Kind]] is not `"initializer"`, _a_.[[Key]] is _b_.[[Key]], and _a_.[[Placement]] is _b_.[[Placement]], throw a *TypeError* exception.


Can this be made more readable? (I don't know if it valid from a spec-grammar point of view)

1. If there are two class elements _a_ and _b_ in _elements_ such that all the following conditions are true: 1. _a_.[[Kind]] is not `"initializer"` 1. _b_.[[Kind]] is not `"initializer"` 1. _a_.[[Key]] is _b_.[[Key]] 1. _a_.[[Placement]] is _b_.[[Placement]] 1. Then throw a *TypeError* exception.

Good idea, done

nicolo-ribaudo · 2018-11-02T16:44:44Z

spec.html

        1. Let _initializer_ be ? Get(_elementObject_, `"initializer"`).
        1. Let _elements_ be ? Get(_elementObject_, `"elements"`).
        1. If _elements_ is not *undefined*, throw a *TypeError* exception.
-        1. If _kind_ not `"field"`,
+        1. If _kind_ not `"field"` or `"initializer"`,


Typo: "If kind is not"

nicolo-ribaudo · 2018-11-02T16:46:46Z

spec.html

+            <ul>
+              <li>_element_.[[Key]] absent.</li>
+              <li>_element_.[[Descriptor]] is absent.</li>
+              <li>_element_.[[Placement]] is `"own"`.</li>


We might consider allowing "static" as the decorator way of doing https://github.com/tc39/proposal-class-static-block

I agree; static should be allowed here since they also can have initializers

I don't see this as a way towards a static block. This is just a way that a decorator can add extra code to run. But decorators are already able to run code at the end of when a class is executing, in the finisher callback. We could allow this out of consistency (it would have slightly different timing from finishers), but I don't see a use case.

A use case is the same as for "own" - being able to invoke a setter on a superclass (in this case, on a superclass's constructor that had, say, static set foo(v) {})

@ljharb What would be the problem with using a finisher for that?

Certainly you could use a finisher for that - but then you'd be forced to use a class-level decorator to change the semantics of a specific field, so instead of:

class C { @useSet static a = x; static b = y; @useSet static c = z; }

you'd need to have:

@useSet('a') @useSet('c') class C { static a = x; static b = y; static c = z; }

or:

@useSet('a', 'c') class C { static a = x; static b = y; static c = z; }

both of which are decidedly less ergonomic and elegant.

I don't see why that would be the case. You can use a finisher on a static field declaration too.

@ljharb, wait, but you can use finisher with a field or a method decorator. It is not required to use class-level decorators to activate finishers.

ahh, that wasn't clear to me. In that case - instead of this "initializer" approach, is there a reason not to allow finishers on "own" fields, to provide an "instance finisher"?

@ljharb yeah, I came to the same idea in my comment below :)

Lodin · 2018-11-02T21:52:57Z

@littledan I have a concern about finisher and initializer field. They kinda duplicate each other. E.g. if initializer fields can be static it almost removes profit finisher brings. If they cannot, it looks strange: why they cannot do it?

Then I came to a thought: why not to do the constructor callback that works exactly as finisher, but during class instantiation? It would reduce not only amount of code that is necessary to describe the logic, but would also be clearer and obvious to use. Or am I missing something?

ljharb · 2018-11-02T21:53:46Z

spec.html

        1. Let _elements_ be ? Get(_elementObject_, `"elements"`).
+        1. If _kind_ is `"initializer"`,
+          1. If _key_ is not *undefined*, throw a *TypeError* exception.


shouldn't this check happen right after Let _key_ be ? Get(_elementObject_, "key")?

I previously tried to sort the validations towards the end, but I can do them mixed inline if you prefer.

I feel like in general we strive to have validations occur as soon as possible, to avoid unnecessary observable behavior.

littledan · 2018-11-02T21:58:07Z

@Lodin I agree that there's overlap between initializers and fields; that's why I started off this PR by saying it was a field with a null key. I also agree that static initializers are not extremely interesting; that's why this patch omits them. A constructor callback could be a fine way to add additional behavior to a descriptor that's already there, but if you want the only behavior to be running the callback (as in the @set example), I'm not sure how we'd make it work.

Lodin · 2018-11-02T22:13:22Z

@littledan Oh I see. My bad, started to read, caught the idea and immediately wrote it. Thanks for explanation.

Well, then, maybe decorators don't need finishers? Initializer fields could be used instead.

littledan · 2018-11-02T22:16:29Z

Finishers are something different, for when the class itself is done being created. They are useful for instance elements as well, e.g., to attach runtime metadata.

littledan · 2018-11-04T17:50:34Z

To @ljharb 's point on the overlap between finishers and initializers: This patch ignores the return value of initializers, but a finisher which returns non-undefined can replace the constructor. The other issue is a subtle one about ordering: All the finishers run after all the field declarations. If we were to try to have initializers subsume finishers, as @ljharb seems to be suggesting above, we'd make the following changes:

Permit both instance and static initializers (and prototype, why not?)
Run the initializers after the field declarations (so it's a three-step process: methods, fields, initializers)
Use the return value for the initializer to replace the object under construction (whether it's the instance or the constructor--or maybe we would ban instance and prototype initializers returning non-undefined?)

(Should we rename initializer to finisher? It's a bit confusing to juggle the term initializer for what this PR introduces and initializer for what a field has.)

rbuckton · 2018-11-04T18:04:11Z

As an example, you could make a decorator to give a field declaration [[Set]] semantics, so the following would work: […]

I still argue that a field should have [[Set]] semantics by default. That said, do we really need this if you can do constructor replacement in a finisher?

ljharb · 2018-11-04T18:55:00Z

Regardless of whether Set or Define was the default, I think you'd want to be able to use a decorator on a single field to have the other semantic, without having to replace the entire constructor and deal with the .constructor and prototype chain difficulties there in.

littledan · 2018-11-04T20:41:36Z

Let's keep the Set vs Define discussion in the class fields repo.

@rbuckton I don't think you would even need constructor replacement for it, just the ability to perform a side-effecting operation (which you would get either from finishers or static initializers).

The above post is about, if we want to try to replace finishers with initializers (as @ljharb suggested above), then how would we do it.

Do you see any part of the above plan that wouldn't provide a replacement for finalizers while enabling the kind of decorator described in the commit message? Would it be too complicated, or unergonomic?

rbuckton · 2018-11-04T20:45:08Z

I am very concerned this will further muddy the waters for the decorators proposal. I think we would be better served to consider this as a follow-on proposal rather than adopt it as part of the current proposal.

In addition I feel it hijacks the semantics of initializer, since for the case where key is not null, the initializer return value is important, while when key is null, the initializer return value is ignored. It very-much feels like we are mixing metaphors and that this could be a source of confusion.

If we were to add this, I would be more willing to accept something like finisher for instances that runs when the current class constructor is evaluated, though not the previously proposed "instance finisher" semantics that waited to evaluate instance finishers until after all constructors (including subclasses) were run.

littledan · 2018-11-04T20:47:44Z

@rbuckton Are you talking about this PR or the plan in #165 (comment) ? The later comment would be like your last paragraph, except the "instance finisher" would run just after all the field initializers and before the constructor body.

nicolo-ribaudo · 2018-11-04T21:13:52Z

How could an instance finisher work in a nice way? I'm trying to understand it using static fields/class finishers, but I'm struggling to implement even simple thigns (replace [[Define]] with [[Set]]): how can I replace a field with a finisher?

This works, but it puts the decorator and the field in different places:

@useSetOn("foo")
class A extends Base {
  static foo = 2;
}

function useSetOn(name) { return _useSetOn.bind(null, name); }
function _useSetOn(name, descriptor) {
  let index = descriptor.elements.findIndex(el => el.key === name);
  let { initializer } = descriptor.elements[index];
  descriptor.elements.splice(index, 1);
  descriptor.finisher = Class => {
    Class.foo = initializer.call(Class);
  };
}

demo

This doesn't work (it throws):

class A extends Base {
  @useSet static foo = 2;
}

function useSet(descriptor) {
  let { initializer, key } = descriptor;
  return {
    finisher(Class) {
      Class[key] = initializer.call(Class);
    }
  };
}

demo

Btw, with finishers instead of initializers this would never work:

class A {
  @useSet foo = 2;
  bar = this.foo;
}

ljharb · 2018-11-04T21:14:39Z

I was envisioning a field decorator able to run an instance finisher as well, to avoid that very problem.

nicolo-ribaudo · 2018-11-04T21:17:13Z

I used class finishers with static fields, but if we had instance finishers I could ask the same question for instance field.

littledan · 2018-11-04T22:23:49Z

@nicolo-ribaudo This is a good point. It seems like I was being too optimistic about one thing subsuming the other. Maybe we should just stick with the own-only initializers proposed in this PR.

ljharb · 2018-11-04T22:27:38Z

If finishers aren’t a solution, then I’m still not sure why this PR wouldn’t cover both own and static.

nicolo-ribaudo · 2018-11-04T22:34:31Z

A possible fix to my point would be to make finishers be declared like normal element descriptors, similarly to the initializers proposed in this PR. The decorator would then become

function useSet(descriptor) {
  let { initializer, key } = descriptor;
  return {
    kind: "finisher",
    placement: "static",
    finisher(Class) {
      Class[key] = initializer.call(Class);
    }
  };
}

However, if I had to choose I'd still prefer initializers, since they allow me to keep the correct ordering in cases like

class A {
  @useSet foo = 2;
  bar = this.foo;
}

nicolo-ribaudo · 2018-11-04T22:37:01Z

@rbuckton

In addition I feel it hijacks the semantics of initializer, since for the case where key is not null, the initializer return value is important, while when key is null, the initializer return value is ignored. It very-much feels like we are mixing metaphors and that this could be a source of confusion.

If key is null, the initializer's return value isn't discarded but it is used to initialize a "null" property:

class A {
  @dec foo = 2;
}

function dec(desc) {
  return {
    ...desc,
    key: null
  }
}

(new A)[null]; // 2

Also move error checks in ToElementDescriptor to happen ASAP

littledan · 2018-11-04T23:10:31Z

@ljharb @nicolo-ribaudo Oh, right. Thanks for bearing with me. The most recent commit permits initializers to be static (or prototype, since why not), and doesn't make any of the other changes mentioned, since we've deduced upthread that they don't make sense. Thoughts?

littledan · 2018-11-04T23:15:56Z

One question remains: Should we look at the return value of the initializer callback, and throw an exception if it's not undefined? This could help avoid programmer errors of the kind @rbuckton may be thinking of, and it could make the non-undefined return value a possible future extension point.

nicolo-ribaudo · 2018-11-04T23:49:25Z

One question remains: Should we look at the return value of the initializer callback, and throw an exception if it's not undefined? This could help avoid programmer errors of the kind @rbuckton may be thinking of, and it could make the non-undefined return value a possible future extension point.

👍 Decorators, even being as powerful as possible, should be as strict as possible to avoid as many potential errors as possible.

ljharb · 2018-11-05T00:11:54Z

spec.html

@@ -563,7 +617,7 @@ <h1>Element Descriptors</h1>
      <p>An <dfn>element descriptor</dfn> describes an element of a class or object literal and has the following shape:</p>
      <pre><code class=typescript>
        interface ElementDescriptor {
-          kind: "method" or "field"
+          kind: "method", "initializer" or "field"


Suggested change

kind: "method", "initializer" or "field"

kind: "method", "initializer", or "field"

ljharb · 2018-11-05T00:12:33Z

spec.html

@@ -689,7 +743,12 @@ <h1>DecorateConstructor ( _elements_, _decorators_ )</h1>
            1. Append _elementsAndFinisher_.[[Finisher]] to _finishers_.
          1. If _elementsAndFinisher_.[[Elements]] is not *undefined*,
            1. Set _elements_ to the concatenation of _elementsAndFinisher_.[[Elements]] and _privateElements_.
-            1. If there are two class elements _a_ and _b_ in _elements_ such that _a_.[[Key]] is _b_.[[Key]] and _a_.[[Placement]] is _b_.[[Placement]], throw a *TypeError* exception.
+            1. If there are two class elements _a_ and _b_ in _elements_ such that all of the following are true:


This reads a bit awkwardly; maybe it should be in an abstract operation?

This is following @nicolo-ribaudo 's suggestion above. I think the factoring might be awkward too; let's work out these editorial issues during Stage 3.

ljharb · 2018-11-05T00:12:47Z

spec.html

@@ -781,26 +842,38 @@ <h1>ToElementDescriptor ( _elementObject_ )</h1>
      <emu-alg>
        1. Assert: _elementObject_ is an ECMAScript language value.
        1. Let _kind_ be ? ToString(? Get(_elementObject_, `"kind"`)).
-        1. If _kind_ is not one of `"method"` or `"field"`, throw a *TypeError* exception.
+        1. If _kind_ is not one of `"initializer"`, `"method"` or `"field"`, throw a *TypeError* exception.


Suggested change

1. If _kind_ is not one of `"initializer"`, `"method"` or `"field"`, throw a *TypeError* exception.

1. If _kind_ is not one of `"initializer"`, `"method"`, or `"field"`, throw a *TypeError* exception.

ljharb · 2018-11-05T00:16:00Z

spec.html

+        1. If _kind_ is `"method"`,
+          1. If _initializer_ is not *undefined*, throw a *TypeError* exception.
+        1. Let _elements_ be ? Get(_elementObject_, `"elements"`).
+        1. If _elements_ is not *undefined*, throw a *TypeError* exception.


Should this use a HasProperty, instead of a Get, since it’s just checking existence? Or is the intention to allow an explicit undefined property - and if so, that seems like it’d be an error, so why allow it?

The Get matches the use of Get when actually using the elements in a class decorator, per POLS.

littledan · 2018-11-05T10:28:09Z

Any further concerns with this patch? @rbuckton It's unclear to me whether we've addressed your concerns.

In my opinion, it's important to get this change in Decorators v1, because I don't want an idiom of "throwaway fields" (used for their side effect) to emerge. I believe it would be very difficult for implementations to optimize away the storage space used in throwaway fields, so I don't want to encourage it in the ecosystem. These have been recommended in various issue threads, and the need for a side effect in this path has been known for more than a year.

littledan · 2018-11-08T22:16:29Z

OK, given the all-around positive reviews, merging this PR. We'll review it in the November 2018 TC39 meeting, and revert or revisit if necessary. Thanks for the help, everyone!

littledan mentioned this pull request Oct 30, 2018

Field declarations overwrite properties on the prototype tc39/proposal-class-fields#151

Closed

ljharb requested changes Oct 30, 2018

View reviewed changes

littledan force-pushed the allow-thunk branch from 71725e4 to 9f6d669 Compare November 2, 2018 16:10

littledan mentioned this pull request Nov 2, 2018

Allow plain thunks to execute as a decorator element #44

Closed

nicolo-ribaudo reviewed Nov 2, 2018

View reviewed changes

littledan added 2 commits November 2, 2018 21:15

Fix most of the errors from review

5bba880

Normative: Throw an exception on a missing initializer

14b8919

ljharb reviewed Nov 2, 2018

View reviewed changes

Normative: Permit prototype and static initializers

9c3c148

Also move error checks in ToElementDescriptor to happen ASAP

ljharb reviewed Nov 5, 2018

View reviewed changes

mbrowne mentioned this pull request Nov 5, 2018

No public instance property declarations rdking/proposal-class-members#1

Closed

littledan added 3 commits November 5, 2018 11:00

Normative: Throw a TypeError if an initializer returns undefined

77543ec

Documentation for initializers

77e1922

Add Oxford commas

9acf8f5

littledan changed the title ~~Normative: Permit null-keyed fields for side effects~~ Normative: Add initializer callback for side effects Nov 5, 2018

mweststrate mentioned this pull request Nov 6, 2018

[on hold] WIP: Support decorators stage 2 proposal, implements #1352 mobxjs/mobx#1732

Closed

23 tasks

littledan merged commit cc2db18 into master Nov 8, 2018

littledan mentioned this pull request Dec 4, 2018

Bikeshedding "initializer" #182

Closed

littledan mentioned this pull request Dec 15, 2018

Change finishers to be normal element descriptors. #177

Closed

tjcrowder mentioned this pull request Dec 29, 2018

@babel/plugin-proposal-decorators doesn't support kind: "initializer" babel/babel#9260

Closed

	kind: "method", "initializer" or "field"
	kind: "method", "initializer", or "field"

	1. If _kind_ is not one of `"initializer"`, `"method"` or `"field"`, throw a TypeError exception.
	1. If _kind_ is not one of `"initializer"`, `"method"`, or `"field"`, throw a TypeError exception.

Normative: Add initializer callback for side effects #165

Normative: Add initializer callback for side effects #165

Conversation

littledan commented Oct 30, 2018 • edited Loading

littledan commented Oct 30, 2018

ljharb left a comment

Choose a reason for hiding this comment

littledan commented Oct 30, 2018

ljharb commented Oct 30, 2018

littledan commented Oct 30, 2018 • edited Loading

rdking commented Oct 30, 2018

ljharb commented Oct 30, 2018

littledan commented Oct 30, 2018

jridgewell commented Oct 30, 2018

rdking commented Oct 30, 2018

rdking commented Oct 30, 2018

littledan commented Oct 30, 2018

jridgewell commented Oct 30, 2018

littledan commented Oct 30, 2018

nicolo-ribaudo commented Oct 30, 2018

littledan commented Oct 30, 2018

nicolo-ribaudo commented Oct 30, 2018

littledan commented Nov 2, 2018

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

nicolo-ribaudo Nov 2, 2018 • 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

Lodin commented Nov 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

littledan commented Nov 2, 2018 • edited Loading

Lodin commented Nov 2, 2018

littledan commented Nov 2, 2018

littledan commented Nov 4, 2018 • edited Loading

rbuckton commented Nov 4, 2018

ljharb commented Nov 4, 2018

littledan commented Nov 4, 2018

rbuckton commented Nov 4, 2018

littledan commented Nov 4, 2018 • edited Loading

nicolo-ribaudo commented Nov 4, 2018 • edited Loading

ljharb commented Nov 4, 2018

nicolo-ribaudo commented Nov 4, 2018 • edited Loading

littledan commented Nov 4, 2018

ljharb commented Nov 4, 2018

nicolo-ribaudo commented Nov 4, 2018

nicolo-ribaudo commented Nov 4, 2018

littledan commented Nov 4, 2018

littledan commented Nov 4, 2018

nicolo-ribaudo commented Nov 4, 2018

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

littledan commented Nov 5, 2018

littledan commented Nov 8, 2018

littledan commented Oct 30, 2018 •

edited

Loading

littledan commented Oct 30, 2018 •

edited

Loading

nicolo-ribaudo Nov 2, 2018 •

edited

Loading

littledan commented Nov 2, 2018 •

edited

Loading

littledan commented Nov 4, 2018 •

edited

Loading

littledan commented Nov 4, 2018 •

edited

Loading

nicolo-ribaudo commented Nov 4, 2018 •

edited

Loading

nicolo-ribaudo commented Nov 4, 2018 •

edited

Loading