Alternate implementation of events/handlers for codesandbox TodoMVC

[geophree]

I (and others) have not been super comfortable with the way crank.js encourages using raw strings in events and event handlers. I re-wrote your codesandbox example using something a little closer to a "action creator" and "reducer switch statement" style of coding, as well as binding directly to the on-attributes of elements instead of using addEventHandler everywhere. I thought it might be useful to show people an alternative without so many addEventHandlers and raw strings.

https://codesandbox.io/s/crank-todomvc-alternative-jluse
https://gist.github.com/geophree/a317c658fd6d7bc51e1e80aa5243be7d

Let me know what you think!

[wmadden]

What's the point of introducing the Redux action/creator function pattern? I can see some value in extracting the magic strings into constants for example, but I think mimicking common React patterns for the sake of familiarity is an antipattern for Crank which shouldn't be encouraged

[geophree]

I think parts of the TodoMVC example are putting a lot of people off, and this is an attempt to show them that (at least some of) the things they are put off by can be easily remedied, and are not integral to crank.js.

There's a lot of boilerplate involved in creating the CustomEvents before bubbling them up. I think using functions to create them, as well as being able to use those functions as values in case statements really cuts down on the number of moving parts and spots where things can go wrong. Combine that with using arrow functions passed in directly to elements on* attributes, and it feels much cleaner to me personally, and it's still using the DOM's event model.

I can see how "redux"-ifying the code could feel counter to crank.js's goals, but I also think it's important not to forget all the good things about the frameworks we've used before. They've lasted so long for a reason.

Also, this is a good example for people to see that crank.js can feel like home for them coming from the frameworks they currently use, and it doesn't have to be a lot more typing and string constants.

[nykula]

I like how you use the function reference as the switch condition. Would start without the ...args thing because it adds more indirection overhead than a POJO payload. And real-world entities tend to have more initial properties than one title, so you eventually add the POJO code path.

What purpose does creatorFunction.type = type; serve?

[mcjazzyfunky]

I like @geophree's version very much - I think this is actually the natural way to go, isn't it?
Sending messages (= immutable data objects) around and delegating the dirty job to somebody else (which you do not even kown) is always a great pattern to achive loose coupling - Redux is only one of many incarnations of this pattern.
And JavaScript does not have algebraic data type constructors or case classes or data classes or whatever you want to call it, therefore action creator functions (or message factory functions) are an obvious and IMHO very nice solution.

Of course @geophree's way will cause more registered event handlers, but so what (same does Preact compared to React)?

What purpose does creatorFunction.type = type; serve?

This is actually a great idea when you are using TypeScript ... with TypeScript this switch (ev.detail.creatorFunction) will prevent the automatic type inference to detect the proper event type within the case clauses, while something like the following should work:

switch (event.type) {
  case Todo.create.type:
      // Here, TypeScript will know exactly that the event is a "todo.create" CustomEvent...
      ...
   case: Todo.edit.type:
      // Here, TypeScript will know exactly that the event is a "todo.edit" CustomEvent...
      ...
}

But finding the correct types to do so could be a bit challenging ....

Last but not least, just as a remark: Of couse, if someone likes to save a few characters you could also use the following API (finding the proper TypeScript types will be even more challenging than with the other API and unfortunately you will have to add some additional information to the event object):

const Todo = defineActions('todo', {
  create: title => ({ title }),
  edit: (id, title) => ({ id, title }),
  destroy: (id, completed) => ({ id, completed }),
  clearCompleted: null
})

[brainkim]

@geophree Looks interesting and I encourage you to continue experimenting! Personally, I’m not a fan of this specific style of programming. At a glance, I’m just not smart enough to regularly think through higher-order functions which reference themselves in their return value. I prefer to write dumb code for my increasingly smooth brain and have left most functional programming tactics behind.

Since you’ve decided to be the delegate for the people who have criticized Crank’s event system, let me respond to y’all, whoever you may be. I don’t understand the criticism that CustomEvent objects are “stringly-typed,” or use “raw strings.” Firstly, is this not true of literally every event system which currently exists? It would be cool to have event types which were symbols to prevent name clashing, but I think the possibility of a clash is overstated and people can get away with prefixing events with a specific app name. Even Redux at the end of the day uses “raw strings.” Actions have a type property, which is just a string oh the horror, and the only reason the shape of the action is as you expect is because you’re expected to use action creators, and an enterprising developer could just call dispatch with the object itself.

You’ve solved this additional “weakness” of Redux by using the actual function as reference, and this feels tempting, but a little analysis of what bugs you were trying to prevent makes me question the value of this approach. The class of bugs which people are worried about is: What if I listen to an event type but receive an unexpected event object/detail? However, insofar as the creatorFunction is available to everyone, you can conceivably create a malformed event with the right creatorFunction, or even mismatch event type and creatorFunction, no?

In short, welcome to JavaScript 😄 Branding your event details with the function which created it makes dispatching events of the wrong shape harder to do, but it also comes at the cost of having to import the Todo helper object anywhere you want to dispatch those events: both parent and child must have exact reference to the Todo object. Maybe this is what you want. Personally, I think the solution to these types of problems isn’t solved in the runtime, but with TypeScript. Currently, the event shapes are only checked at the addEventListener side, but I have some ideas on how to check it at the dispatchEvent side as well, and just haven’t gotten around to figuring it out.

Finally, this isn’t just Crank’s event system but the event system of the actual browser. The “boilerplate” you see is exactly what you’d need if you were doing vanilla JavaScript. I firmly believe in adopting conventions wherever possible, and Crank‘s EventTarget system tries to behave exactly as the actual EventTarget system does, except it operates at a component level. This makes the things you learn in Crank transferable to vanilla JS and vice versa.

So when you criticize Crank’s event system, you’re actually criticizing the browser’s, which to be honest, is fair. But I’d much rather mimic an API and have people abstract on top of it if they feel it’s too “low-level,” as opposed to doing something different and forcing people to learn two different APIs. I think your example shows that this sort of abstraction is possible, and I encourage you to experiment! I’m a big fan of not telling people how to write their code when I don’t have to personally deal with it, and if this is what gets you excited, I am onboard. Just sharing my thoughts as a fellow developer is all.

[mcjazzyfunky]

@brainkim I think the most important part in @geophree example is the fact that you can write this (patter 1):

function* Counter() {
  let count = 0

  const onClick = () => {
    ++count
    this.refresh()
  }

  while (true) {
    yield (
      <div>
        <label>Counter: </label>
        <button onclick={onClick}>
          {count}
        </button>
      </div>
    ) 
  }
}

instead of this (pattern 2):

function* Counter() {
  let count = 0

  this.addEventListener('click', ev => {
    if (ev.target.tagName === 'BUTTON') {
      ++count
      this.refresh()
    }
  })

  while (true) {
    yield (
      <div>
         <label>Counter: </label>
         <button>{count}</button>
      </div>
    )
  }
}

I prefer "Pattern 1" by far but honestly until I've seen @geophree's TodoMVC implementation I've always used "Pattern 2" just because stupid me really thought "Pattern 2" is the (one and only) way to handle events in Crank, because in all demos I've seen it's done that way.

Are there any important reasons why you prefer "Pattern 2" to "Pattern 1" or is it just a matter of taste (what would be perfectly fine, of course)?

[brainkim]

@mcjazzyfunky I updated the docs to reflect the “Pattern 1” in https://crank.js.org/guides/components, but you may not have seen it. It definitely is a matter of taste. I like “Pattern 2” for reasons I detail in the following comments:

#10 (comment)
#10 (comment)
#57 (comment)

I’ll keep defending Pattern 2 but both styles of API are available.

[mcjazzyfunky]

@brainkim Maybe you've catched me at not being good in reading the f****n' manual .... 😄
Thanks for hyperlinking that comments, they are really enlightening.

[mcjazzyfunky]

@brainkim BTW (a bit off-topic - a copyright question): I have this little web components toy library and I have taken you're TodoMVC demo and replaced all Crank components by custom elements.
Do I have your permission to add that modified "crankless" TodoMVC demo to the github repository of my toy library? Thanks...

[brainkim]

@mcjazzyfunky Yes of course! Everything is hopefully licensed under the MIT license and if there are copyright issues let me know.

[geophree]

Updated with better-named link and gist link.

https://codesandbox.io/s/crank-todomvc-alternative-jluse
https://gist.github.com/geophree/a317c658fd6d7bc51e1e80aa5243be7d

[geophree]

I think we're going to have to agree to disagree on this one. At least now you can point people here when they complain about it! Thanks again for making such an interesting framework.

[brainkim]

@geophree I sense a religious war brewing. And I’m not sure I’ll win 😄

[geophree]

When the 10th-generation EventFactory-onevent (a.k.a. F1-VENT) and CustomEvent-addEventHandler (a.k.a CEE-Ha!) factions decimate each other and the remnants are brought together by their common love of this.dispatch and form the 'PATCH Alliance maybe all us reasonable old-timers can grab a drink, if we manage to survive... 😆

[mcjazzyfunky]

The following may be a bit off-topic:
Like said somewhere above I share @geophree's opinion that action creators are often very helpful.
I mentioned yesterday that while the implementation in JavaScript for some action creator helper functions may be quite simple, proper typing of those functions with TypeScript is far more challenging.
Since yesterday I've spent quite some time to decide what's my personally preferred API and typing solution for those action creator helper functions and if you allow, I would like to share that with you now (spoiler: It's basically the one that @geophree is using).

If you are using action creators in TypeScript and also are interested in how they could be typed properly, then maybe the follwing could help you a bit - if not, it will surely bore you to death 😃 ... so feel free to quit at this point 😉 .... anyway...

The main challenge regarding proper action (creator) typing is that if you have an action like follows

{
  type: 'todo.add',
  ...
}

then the type of property 'type' must not be just string but exatly 'todo.add' as in

const action = {
  type: 'todo.add' as const,
  ...
}

Otherwise TS type inference will not work exactly as you want.

If you define your action creators the way @geophree did, proper TS typing will be possible:

const Todo = {
  create: defineAction('todo.create', (title: string) => ({ title })),
  edit: defineAction('todo.edit', (id: number, title: string) => ({ id, title })),
   ...
}

If it's okay in your use case that the type names are not like 'todo.create' but just like 'create', the following is also properly typeable with TypeScript (as an alternative you could of course use constants for the keys/type names - but I personally prefer a shorter version without explicit constants declaration - nevertheless the demo linked below contains one example using type name constants):

const Todo = defineActions({
  create: (title: string) => ({ title }), // value of prop 'type': 'create'
  edit: (id: number, title: string) => ({ id, title }), // value of prop 'type': 'edit'
  ...
})

But the following solution is NOT properly typeable with the current TS typing features (this is basically the alternative I've mentioned somewhere above, with the remark that there will be a lot of trouble with the typing of that):

const Todo = defineActions('todo', {
  create: (title: string) => ({ title }), // value of prop'type': 'toto.create'
  edit: (id: number, title: string) => ({ id, title }), // value of prop'type': 'todo.edit'
  ...
})

The reason that the latter API is not properly typeable is that with the current TS type system you cannot build programmatically a new type'todo.create' out of two given types 'todo' and 'create'. You would have to use the type string which is too general for our purpose.

Some workarounds like additional properties for the action object could help

{
  type: 'todo.create', // type: string
  group: 'todo', // type: 'todo'
  kind: 'create', // type: 'create'
  ...
}

but this would be really confusing and at the end of the day IMHO that's not a good solution.

To make a long story short: Use that action creator pattern that @geophree is using in his example 😉 (at least if you are using TypeScript)

Please find here a demo with all mentioned patterns that are properly typeable (be aware that the actions/messages in this demo are not DOM CustomEvents but just simple data objects - but that does not really make a difference here).
Be aware that the important part of that demo is, that the types of the variables action, action2 and action3 are always 100% perfectly inferred in each of the case clauses (hover over each occurrence of those three variables and check the type).

https://codesandbox.io/s/priceless-curran-xjbcz?file=/src/index.ts

Thanks again @geophree ... your TodoMVC implementation was really a big help.

[mcjazzyfunky]

Hope nobody minds me to write again a few words on this topic (I know it has been closed three weeks ago :-) ).
In my personal opinion most of the things that were discussed in this thread have nothing to do with Crank itself, but more about how to use various patterns on top of Crank.
Redux for example has basically nothing to do with React, it's just one of many state management libraries that can be used with almost any UI library.

What really bugs me a bit, is that we in the Crank community (same problem for other communities - but we are the cool community 😉 we should do better) seem to have a light tendency to mix statements that describe facts with statements that describe just personal opinions or taste. And quite often topics of personal taste are labeled as advantage of Crank itself.

For example @geophree has mentioned somewhere above that using these addEventListenerfunction is (quote) "putting a lot of people off": I can only speak for myself, but at least I am indeed one person who was kinda "put off" the use of those addEventListenerfunction. "Why should anybody ever want to do this?!?" was my first thought" ... I believe that you, Brian, prefer this style of event handling, but until you really find a way to proof why it is better, it is just a matter of taste not more (I think I've read somewhere that you are planning to write a little blog article on this topic ... looking forward to that ... who know's maybe you can convince me ;-) 👀...)

And under really NO circumstances I'll believe that using Redux or pure functional state transitions in general is an anti-pattern for crank or any other UI library. Claiming that something that's impure, not really properly typeable, hard to test and that might even result in more lines of code is better than using mostly pure and perfectly testable functions and patterns that can be typed 100% properly in TypeScript is really really ambitious. I really understand that many in the Crank community prefer writing components without additional libraries, but that's fine for simple components and small project, but things will change quickly if the project increases in size, IMHO.

Please have a look at a modification of @geophree's TodoMVC demo.
As it's in JavaScript you cannot really see it, but both the action creators and the reducer are 100% pure functions and everything is 100% typesafe (and this experimental update function will still be type-safe for deeper nested data structures).

It's just an opinion whether you like or hate this kind of programming, but it's a fact, that the main part of the demo is shorter than the original one (in the Crank repo), that things are better typeable, that the state transitions are way better testable and that there's a better separation between UI logic and business logic.

And btw: Yes it's true that you (quote see above) "hav(e) to import the Todo helper object anywhere you want to dispatch those events" as this is part of the public API of the business logic.

(of course this send and receivefunctions are a bit of a hack and the App component in is a bit quick'n'dirty implemented - but I think that's not really important here):
https://codesandbox.io/s/crank-todomvc-alternative-66e5q

PS: @geophree I've used the shorter syntax for the action creator definitions here, but only to prove myself wrong that this was not typeable in TypeScript. You'll have some trouble in switch (action) { ... } statements but if you use it in this "ngrx"ish way, it's yet properly typeable. Nevertheless, I still think, the way that you have defined the action creators is the better one.

[mcjazzyfunky]

Okay, something else: I've taken this Crank+Redux TodoMVC demo from above:
https://codesandbox.io/s/crank-todomvc-alternative-66e5q

and converted it to React (franky I've cheated a tiny little bit to make it under 300 LOC ;-) ):
https://codesandbox.io/s/todomvc-react-tu8ev

I'm quite sure there are some bugs in both versions but for a simple comparision that's fine, I think.

It's interesting that the React version is 300 to 500 characters longer (depending on how you count) and the Crank version will even be shorter as soon as a "crank-router" package will be available.

My personal opinion: When you implement something in Crank and after that you implement something in React then these useCallback(..., [....]) thingies feel really annoying - but the rest is quite okay IMHO, different but okay.