Crank: Question regarding documentation: Chapter "Reusable logic"

[mcjazzyfunky]

@brainkim

I guess what's written in chapter "Reusable Logic" (especially under section "Global Context Extensions" and "Context Helper Utilities") is Cranks answer to React hooks, isn't it?

Two questions:

1. Regarding "Global Context Extensions":

   Isn't that in 99,9% of all cases a complete anti-pattern?
   Why should I cause confusion by writing

   Context.prototype.mySuperfancyExtension = ...
   
   interface Context<...> {
     mySuperfancyExtension(...): ...
   }
   

   when I can implement the same functionality with a slightly different usage syntax

   function mySuperfancyExtension<...>(ctx: Context<....>, ...) { ... }
   

   without causing any trouble and confusion at all???

   BTW: Extending prototypes is just a standard JavaScript feature, not a Crank feature, why even mention it in the documentation?

2. Regarding "Context Helper Utilities":

   Do component implementations like the following (which use "context helper utility" functions, here named withXYZ(ctx, ...)) officially have your (@BrainKim's) blessing now or are they still NOT considered proper Crank style:

   function* Metronome({ bpm = 60 }) {
     let tock = false
   
     withInterval(this, () => {
       tock = !tock
       this.refresh()
     }, () => 60000 / bpm)
   
     for ({ bpm = 60 } of this) {
       yield (
         <div>
           Metronome ({bpm} bpm): {tock ? 'Tock' : 'Tick'}
         </div>
       )
     }
   }
   

   function* Loader() {
     const loader = withPromise(this, () => wait(4000))
   
     for ({ loadingText, finishText } of this) {
       if (loader.getState() === 'pending') {
         yield <div>{loadingText || 'Loading ...'}</div>
       } else {
         yield <div>{finishText || 'Finished!'}</div>
       }
     }
   }
   

   Here's a running demo

[brainkim]

Regarding "Global Context Extensions"… Isn't that in 99,9% of all cases a complete anti-pattern?

I say in the docs that global context extensions should be limited to well-known APIs. In my mind, I think having extensions like this.setInterval and this.requestAnimationFrame are useful because we can access them without shadowing globals in the module scope, and still preserve the same API names and signatures. You can define alternative names, but people might not know how to call withInterval at a glance, especially given that it takes two callbacks.

BTW: Extending prototypes is just a standard JavaScript feature, not a Crank feature, why even mention it in the documentation?

It’s mentioned in the documentation as one of four alternatives, and there are big caveats about how you should reserve the technique for well-known, global APIs. I don’t see the harm of mentioning it as a possible pattern.

Do component implementations like the following (which use "context helper utility" functions, here named withXYZ(ctx, ...)) officially have your (@brainkim's) blessing now or are they still NOT considered proper Crank style

Cool demo! I’m just happy your code hasn’t surfaced any obvious bugs to be honest. I’m not sure about the deps array/callback pattern, and I’m not sure about some of the APIs, like it’s unclear what the benefits of the withPromise function are over using async/await directly. Maybe the fact that you can easily swap the loading and finished messages without rerunning the async function, which I think might be a little harder to do with async generator functions. However, the demo does seem to provide interesting ideas and I’m happy to see where it leads. I dunno if I can give my “blessing” haha, because the APIs do feel a little hooky and I don’t really know yet whether this style of API produces good code, but I appreciate the experimentation and think the actual produced demos are pretty cool.

On a related note, it’s an absolute shame that React has decided to squat on functions starting with use for all perpetuity, because I think useX is much preferable to withX in terms of naming, but what’s done is done.

[mcjazzyfunky]

@brainkim Many thanks for your answers.

I’m not sure about some of the APIs, like it’s unclear what the benefits of the withPromise function are over using async/await directly.

This Loader component is basically the same example that I have used in #84.
The problem still is that for a non-trivial larger component it's quite common, that data fetching is based on e.g. certain props (data have to be reloaded if those props change) and some view aspects depend on some other props that have nothing to do with the data fetching (=> therefore see the third argument (= getDeps) in withPromise and withEffect). So often when props change it's important that the view has to be updated but the fetch/loading state must not be efffected. As async generators are "blocking" while "await"ing I have chosen to go for a sync generator component.

@CrankCommunity (hopefully some folks are reading this): If someone manages to replace this synchronous generator component Loader in my demo with a nice async generator component which behaves exactly the same, that would be really great and would help a lot to evaluate and determine proper cranky design/implementation patterns.

[brainkim]

@CrankCommunity (hopefully some folks are reading this): If someone manages to replace this synchronous generator component Loader in my demo with a nice async generator component which behaves exactly the same

It’s just you and me haha.

If I understand your requirements correctly, shouldn’t this be equivalent?

function* Loader({loadingText="Loading...", finishText="Finished!"}) {
  let state = "pending";
  wait(4000).then(() => (state = "fulfilled"), () => (state = "rejected"));
  for ({loadingText="Loading...", finishText="Finished!"} of this) {
    if (state === "pending") {
      yield <div>{loadingText}</div>;
    } else {
      yield <div>{finishText}</div>;
    }
  }
}

This lacks the sophistication of your code, but typically, I find falling back to promise callback methods directly when I’m doing advanced concurrency stuff is fine. I’m still not sure I understand the larger use-case. Maybe a more concrete real-world example would help.

[mcjazzyfunky]

@brainkim

It’s just you and me haha.

Hopefully not only you and me are notified automatically of ALL conversations in the Crank project. :smiley:

If I understand your requirements correctly, shouldn’t this be equivalent?

Yes, except for the missing this.refresh() invocations, that's working fine.
But frankly, the challenge was not to replace this withPromise function but to replace the sync generator component by a async one (while still working properly and hopefully still be more or less concise).

In my simple example the "fetch/load" function (which was a simple wait here) did not depend on any props, but normally you have something like fetch(`https://services.xyz/user/${props.userId}`) and whenever the userId prop changes the data fetching must restart, while it must not restart if any other prop changes. This is where the third argument getDeps of that withPromise function shines (which has not been used in my simple example above).

I’m still not sure I understand the larger use-case. Maybe a more concrete real-world example would help.

Let's assume in one year Crank is completely established in the world of UI libraries and you decide to write a material design component suite for Crank (similar to Vuetify or whatever). Then you decide that all of your components (or at least where it is useful) shall have a property cssClass (or name it class or className if you prefer) where the className DOM property of the surrounding div can be defined.
Of course it's important that as soon as this cssName property changes the component will be refreshed within milliseconds.
If you have a async generator component which has such a cssClass property then it will not be (easily) possible to guarantee that changes of prop cssClass will be commited within milliseconds as it's possible that the async component is currently awaiting for several seconds and blocking the whole thing. Am I right?

I claim that something like this (and this has the real-world pattern "data fetching depends on some props, the view depends on some other props") will be remarkable challenging to be implemented with async generators and I've really asked myself since monthes now: Are there really non-trivial use-cases where async generator components completely shine while the implementation with "normal" generator components would completely suck?

function* QuoteOfTheDay({ locale }) {
  const loader = withPromise(this, () => fetchQuote(locale), () => [locale])

  for ({ locale, cssClass } of this) {
    yield (
       <div class={cssClass}>
         {loader.getState() === 'pending' ? <Spinner/> : loader.getResult() || loader.getError() }
       </div>
     )
  }
}

[nykula]

Hopefully not only you and me are notified automatically of ALL conversations in the Crank project.

Off-topic: I'm scrolling all conversations and commits to understand the development but I have few comments (: For my use, Crank in its current form is okay.

[mcjazzyfunky]

Just as a final remark:
It's of course perfectly fine if the patterns I've used in my demo above are too (quote) "hooky" and those "deps array/callback pattern" are too React-like to be considered as useful Crank patterns (and therefore should be avoided). Nevertheless I personally like it very much to noodle around with alternative design patterns. Therefore just in case someone - like me - also likes to see alternative solutions, please find here an update of my demo above.

See: DEMO2

What may or may not be interesting is the following (most of that I have already mentioned somewhere at other issues):

• All components are "normal" function components - no generator or async components used in these demos
• Provides a concrete counterpart to React hooks. I've named those functions "extensions" and their function names follow the pattern withXYZ.
• Shows a provision API that allows more or less the same behavior as with React contexts: API looks like const [provideTheme, consumeTheme] = defineProvision('light')
• Provides an alternative solution for handling default props (see withProps)
• Loading data asynchronously without using async components (see withPromise)
• Auto-refreshing on state change (see withState - I'm personally not a big fan of this solution - but it's just about showing alternative approaches, so why not?)
• Alternative way to handle intervals (see withInterval)

[brainkim]

@mcjazzyfunky

Nevertheless I personally like it very much to noodle around with alternative design patterns.

Yeah I’m just giving my personal thoughts, and I appreciate all the experimentation, if only because you’re exercising the codebase and discovering potential patterns that I hadn’t thought about. Please don’t assume I’m the authority on good code and feel free to completely disregard everything I say; I would never want to hamper a fellow developer’s freedom of expression.

All components are "normal" function components - no generator or async components used in these demos

I was afraid this would happen, but it was probably inevitable if we want to support any kind of plugin architecture. I really don’t want to run into the situation React has where hooks are consuming the entire ecosystem, so that you now have hooks like useModal or useTable, APIs which would ordinarily have been exposed as components rather than hooks.

The design decision which I used to firewall the component abstraction and protect it from plugins is to say that only components are able to respond to updates and yield values. The callbacks passed to schedule and cleanup only ever respond to committing/unmounting; the updates are still always handled by the component itself. You can think of it as the top-level component code being the main thread, while the nested callbacks or plugin logic happens elsewhere and support the main logic of the component (I know JavaScript is single-threaded).

I think there is some promising stuff in the provision API you showed, especially combining provisions with the event system seems interesting.

[mcjazzyfunky]

@brainkim

All components are "normal" function components [...]

I was afraid this would happen.

The reasons why I gave that a try were:

• The Crank doc says we use generator functions for stateful components - the examples in Demo2 above show that you can also use "normal" functions for stateful components (what does not necessarily imply that you should ever do so, of course :wink:).

• Sync generator components support counterparts for the following React lifecycle metods componentWillMount, componentWillUpdate, componentWillUnmount, componentWillCatch and also that useLayoutEffect lifecycle stuff out of the box without the need to call a special Crank API function. This is of course extremely interesting and one reason why so many developers love Crank. But unfortunatelly I personally would like to use counterparts for componentDidMount and componentDidUpdate which is only possible in Crank with sync generator components by using this.schedule(...). As I REALLY want to use some counterparts of componentDidMount or componentDidUpdate the only option I have (or at least the only option I know) is to use something like that withEffects extension that I've used in my demos - but something like that is (like we both have agreed) not proper Crank style, so what shall I do?

• It's kinda puzzling to me that nobody has ever mentioned that the way default props and props destructuring are handled in Crank (at least reagarding to the Crank doc) may not be a very elegant solution (frankly: to say the least). What happens if a component has 20 props most of which are used for both the view and for side effects. Do you still destructure all that stuff twice and do you also declare the default props twice (btw: you normally have to list all props again when defining the componets props type in TypeScript)?
  And for example there are a lot of traps because you have to update those destructed prop values explicitly all the time (same pattern that I always mention as a common trap in Crank).
  For example it may be difficult to find the bug in the following code (and if you do not agree that it may be difficult as this simple example is quite short, if your component has 20 props and hundreds of lines of code it might be):

  function* QuoteOfTheDay({ locale }) {
    const loader = withPromise(this, () => fetchQuote(locale), () => [locale])
  
    for ({ cssClass } of this) {
      yield (
         <div class={cssClass}>
           {loader.getState() === 'pending' ? <Spinner/> : loader.getResult() || loader.getError() }
         </div>
       )
    }
  }
  

  Spoiler: Of course it has to be for ({ locale, cssClass } of this) { ... }.

  Frankly, I personally do not like that kind of props/default props handling at all. Maybe I would do something like the following instead (also not a very elegant solutions, but atually I do not have a good/better idea), but again the problem is that this is not proper Crank style, so again: What shall I do?:

  const counterDefaults = {
    initialCount: 0,
    label: 'Counter'  
  }
  
  function* Counter1() {
    // props will be mutating automatically, properties will always be up-to-date 
    const props = withDefaultProps_v1(this, counterDefaults)
    ...
    while (true) {
      ...
      yield ...
    }
  } 
  
  // Actually I would prefer the above solution to this (some not-so-often used lifecycle interceptions
  // are not possible with "normal" functions etc.), but anyway...
  function Counter2() {
    // return value of getProps will be immutable
    const [getProps, view] = withDefaultProps_v2(this, counterDefaults)
    ...
    // argument props will be immutable
    return view((props) => {
      ...
      return ...
    })
  } 
  

I really don’t want to run into the situation React has where hooks are consuming the entire ecosystem,

React hooks are powerful indeed, but I think, it's not their fault that hooks are "abused" very often. With power comes responsibility, and here the custom hook authors are responsible not to do stupid stuff.

[brainkim]

@mcjazzyfunky

But unfortunatelly I personally would like to use counterparts for componentDidMount and componentDidUpdate which is only possible in Crank with sync generator components by using this.schedule(...)

Yeah this is a consequence of the fact that sync generators suspend at their yield, unlike async generators. I tried to think of a possible double rendering scheme which allowed code to run after yields, but I don’t think it’s possible. The schedule method seemed like the least bad solution, and it allows for interesting idioms like this.schedule(() => this.refresh()) right before the first yield to emulate componentDidMount logic in sync generators.

It's kinda puzzling to me that nobody has ever mentioned that the way default props and props destructuring are handled in Crank (at least reagarding to the Crank doc) may not be a very elegant solution (frankly: to say the least).

Haha there are at least three separate issues (#13, #26, #41) proposing alternate APIs to the props system I proposed. None of them picked up on the default props mismatch stuff; most of the criticism is motivated by this-phobia. I think you’re definitely right that it’s not optimal, but I maintain that it’s the least sub-optimal API given the past of JSX (React function components), TypeScript typings, and flexibility. I wanted to optimize function component -> generator component refactors, because so often we start with stateless components and later add state, and turning a function component into a generator component just means copying the props parameter into a for statement. Additionally, you forget that it’s sometimes nice to have divergences in the props, because this allows you to compare old and new props.

[mcjazzyfunky]

Well, we have to admit enviously that our friends of the class community do not have such problems ... :grin: :smile: :joy:

@component('Counter')
class Counter extends StatefulComponent {
  @prop()
  initialCount = 0

  @prop()
  label = 'Counter'

  *main() {
    let count = this.initialCount

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

    while (true) {
      yield (
        <button onclick={onIncrement}>
          {this.label}: {count}
        </button>
      )
    }
  }
}

export default crankify(Counter)