nettybun / haptic

Explicit reactive web rendering in TSX with no compiler, no virtual DOM, and no magic. 1.6kb min+gz.
MIT License
79 stars 3 forks source link

Syntax too cryptic? #5

Closed mindplay-dk closed 3 years ago

mindplay-dk commented 3 years ago

I know it's early days for this project, and perhaps too soon to critique anything - but I'm following along, as I've been very interested in Sinuous for a while now, and this promises to be a "rewrite of Sinous", and I would love to see those ideas evolve from here. πŸ™‚

I'm looking at the example, and frankly I'm finding it a bit obscure. One of the things Sinuous has going for it, is it's reasonably intuitive to get started with - I really wouldn't like to see that get lost in a rewrite.

So here's my (full disclaimer:) really opinionated comments, take'em or leave'em. 😁

import { wS, wR, v$ } from '../src/wire/index.js';

☝ These functions need proper names.

I think I pointed you to dipole already? This has fairly intuitive naming, once you're used to the terminology: observable(value) creates an observable value, computed(() => { ... }) creates a computed reaction, reaction(() => { ... }) creates reactive effects, and so on.

I also like that Dipole's observables make reading and writing explicit with .get() and .set(value) methods - it requires slightly more work on the keyboard, but makes it much easier to skim through a large chunk of code and spot where the reads/writes are taking place. Opinionated, but readable code is more important than writable code: you only have to write it once, but you have to read it many times, your coworkers have to read it, and so on.

const data = wS({
  text: '',
  count: 0,
  countPlusOne: wR(($): number => {
    console.log('Conputing countPlusOne');
    return data.count($) + 1;
  }),
  countPlusTwo: wR(($): number => {
    console.log('Conputing countPlusTwo');
    return data.countPlusOne($) + 1;
  }),
});

☝ These APIs need to be consistent.

Why does wS accept an object with many observable values, and reactions mixed in?

The text and count observables, and the countPlusOne and countPlusTwo reactions, do not have any relationship with each other, as far as I can figure? I don't think there's any compelling reason to pack unrelated initializations into a single function - so I'd prefer these two functions were more consistently both singular factory functions, something like:

const data = {
  text: observable(''),
  count: observable(0),
  countPlusOne: reaction(($): number => {
    console.log('Conputing countPlusOne');
    return data.count($) + 1;
  }),
  countPlusTwo: reaction(($): number => {
    console.log('Conputing countPlusTwo');
    return data.countPlusOne($) + 1;
  }),
};

This is already more readable, consistent and intuitive - it should simplify typing with TS as well.

    <p>Here's math:
      {wR(($) => data.count($) < 5
        ? Math.PI * data.count($)
        : `Text: "${data.text($)}" is ${data.text($).length} chars`)}
    </p>

☝ Too many dollar signs πŸ’Έ

I think I get what you're trying to do here, and I love the fact that the reactive context isn't somehow established by some magic behind a dark curtain.

Side story: I recently made a friend try out Sinuous, and the magic actually bit him. He had a parent component that contained a reactive expression - and in a child component that was used in that reactive expression, he thought he could safely read from an observable, in a section of the child component's code that wasn't reactive. Which you absolutely can - but the reactive expression in the parent component will pick up the usage and associate it with updates of the parent, which caused unexpected updates. It took him a very long time to fix this. That's nasty and frustrating learning curve, and it's something anybody is likely to run into at first.

Dipole has the same problem, btw.

Fixing that would be awesome.

But the syntax here is a bit ugh - manually passing that same argument again and again everywhere.

Also, you end up unnecessarily reading the same observables more than once. (which, yeah, you could assign the values to variables first, but then almost nothing dynamic would ever be just a single, elegant expression, so, ugh.)

Did you consider maybe something like this instead?

    <p>Here's math:
      {reaction([count, text], (count, text) => count < 5
        ? Math.PI * count
        : `Text: "${text}" is ${text.length} chars`))}
    </p>

This is even more explicit - though perhaps too explicit, and creates subscriptions on things that might not actually be used, so, meh. You might could get around that with even more explicitness:

    <p>Here's math:
      {reaction([count], count => count < 5
        ? Math.PI * count
        : reaction([text], text => `Text: "${text}" is ${text.length} chars`)))}
    </p>

This gives you the right subscriptions but, eh, that's not pretty.

Yeah, I'm not really sure what to suggest here. πŸ€”

And this last thing:

    <input value={wR(data.text)}/>

☝ I know that's actually the same operation as the other reaction - that data.text is the reader function, and it'll receive the $ argument, so it's equivalent to the redundant wR($ => data.text($))... but it looks like two completely different things.

I know Sinuous does something similar, allowing observables to be used directly in expressions, where it'll create the reaction for you, but it's not a favorite feature of mine either...

I like consistent patterns in UI libraries - where you recognize operations intuitively, without parsing.

I don't know, maybe this is yet another thing you just have to get over - but it's worth thinking about learning curve and adoption rate. Not at all cost, of course - but it's not as much fun being the one guy who gets something, going through life struggling to get other people to get it, and I think that's really the key to React's success, and certainly to the introduction of hooks, even though those are absolutely horrendous and nasty behind the curtains.

I wouldn't want to trade off good or clean or correct for something magical that "looks cool", but is there something we can do to make this more intuitive and palatable?

I hope you don't find any of this too offensive, but feel free to tell me to f_ck off and mind my own business. πŸ˜„

nettybun commented 3 years ago

Thanks for the write up - lots to unpack and respond to but having a critique means a lot. I'll message back soon :)

mindplay-dk commented 3 years ago

Here's something I thought of this morning.

Since you like grouping states into "container" like structures, how about something like this...

const data = container({
  text: '',
  count: 0,
  countPlusOne: (({ count }): number => {
    console.log('Conputing countPlusOne');
    return count + 1;
  }),
  countPlusTwo: (({ count, countPlusOne }): number => {
    console.log('Conputing countPlusTwo');
    return countPlusOne + 1;
  }),
});

How this might work:

It looks nice, but what do you think, could that work? πŸ€”

nettybun commented 3 years ago

Hi thanks again for feedback about the project. I've definitely been at it too long and can't see the forest from the trees anymore; things feel intuitive to me but I can only guess how it comes off to others. Also, you're right that it is in the early stages, and I didn't want people to see the "examples/index.tsx" as an intro or an example - it's only my syntax testing area to make sure it doesn't crash and TS is happy - but I have no docs/intro yet so it's fair that you're critiquing it.

I'll make a real intro and move the syntax testing off the repo. I think that's the right option for working in public, otherwise people get confused and that's totally fair.

I've made a lot of design decisions through evaluating different options and choosing the least bad of them, sometimes I'll have big code comments about that, but I've meant to be better about documenting how/why those decisions are made. Your comment is a good opportunity to start that.

I'll address your message bit by bit.

... and this promises to be a "rewrite of Sinuous" ... I really wouldn't like to see that get lost in a rewrite.

I've actually never mention trying to be a rewrite or replacement of Sinuous πŸ˜… That's a non-goal! I love Sinuous, and I think it's doing great. We just have different goals: I focus on TypeScript, JSX, and having as-little-code-as-possible; Sinuous is into JavaScript, HTML tag templates, and having a lot of additional packages in a monorepo like sinuous/{data, template, map, hydrate, etc}.

I see this in Solid too, which has a lot of packages linked together across different repos, npm packages, and code editions - it's a lot of code to search through and understand.

☝ These functions need proper names.

They have proper names actually. I import the shorthand because I'm me, but they're actually exported as wireSignals and wireReactor. Also, they have JSDoc comments so you should be able to hover over the element and it'll talk about it.

image

Speaking of making a proper intro, $v is more of an edge usecase, when you're working with a large team or interacting with code you haven't written or don't trust. It shouldn't be in an intro/example. I'll talk about naming below when I talk about the choice of "$" tokens.

I'll consider a more descriptive name like createNamedSignals() and createReactiveContext() but as part of the "no magic" rule I need reactive expressions to be explicit, and in JSX that means: <p>There are {items} items</p> isn't good enough. There has to be a reactor. I actually changed haptic/dom to serialize functions specifically so it's clear that passing {items} is wrong and won't silently fail - it'll print the whole function body to the page. That's why I chose wR because it's small. You could use the full version...

<p>The text is {wireReactor($ => data.text($))}</p>

But... I don't know if I can sell that in a way people will use. In a intro/readme I'll explain the long form first and then say "Don't worry you can shorten this to just wR(data.text)".

I think I pointed you to dipole already? This has fairly intuitive naming ...

Yeah it's a pretty common naming schema. Sinuous does it too. I have pretty much the same thing, observables ➑ signals, reaction ➑ reactor (which I get from Reactor.js). I specifically don't have a third "computed" type; it's confusing to people that in other reactive libraries a computed is kind-of-a-signal in that you can read/sub it but you can't write to it. Even in my initial port of Sinuous' engine I called them "WritableSignal" and "ComputedSignal". In Haptic Wire a computed-signal is literally a signal. There's only ever one signal type, and I know I haven't described that very well. One sec...

☝ These APIs need to be consistent. Why does wS accept an object with many observable values, and reactions mixed in? The text and count observables, and the countPlusOne and countPlusTwo reactions, do not have any relationship with each other, as far as I can figure?

Yeah again I need to intro what's happening here. Thanks for calling it out! So there's actually a super important reason for packing it as a single object; signals/reactors have names! 😁 Like real names! They appear in stack traces and make debugging super easy in the console.

2021-04-27_14-36

Useful too when looking into a reactor:

2021-04-27_14-38

Especially lovely for errors

2021-04-27_14-41

I haven't seen anyone else do this before.

I get that you like const count = observable(0) but due to JS evaluation order there's no way to know the name unless you do count count = o(0, 'count') which I don't think anyone will want to do. I want people to have autocomplete too, and having a single object gives them that - they can type "data." and the editor shows them all the signals available. I'm also against your computeds-as-functions proposal for this reason too, I don't think you can unpack the signals into a function during its own initialization and have successful autocomplete due to limitations of the TS compiler https://github.com/microsoft/TypeScript/issues/43683.

You mentioned getting bit by signals. When an error throws or something isn't linked up right, it's so important that you have names for free. No work! No compiler hacks (like https://github.com/luwes/sinuous/issues/78) to inject a name!

So about computed-signals and your question about seeing reactors in the wireSignals definition/initialization block. PR here https://github.com/heyheyhello/haptic/pull/1 this is honestly some magic. Feels bad, but I want people to be able to write a function as a value into a signal and not have anything odd happen; I have no interest in intercepting a random function. Just let it through. So if I need to introduce special behaviour it has to be with a data type I own - I only have signals and reactors, and it's important that there's only one type of a signal to avoid confusion: wireSignals only wires signals! Reactors are computation engines, so it followed that passing a reactor into a signal creates a computed-signal. That's the magic. Maybe it's bad. Maybe I'll keep it out of the intro and into the "Advanced" docs. Maybe I should have a dedicated createComputedSignal but at the end of the day it's still going to be a normal WireSignal signal, which means you can write a value to it. If I write "10" to a computed-signal, the computed (signal+reactor) is gone, and it's now just a container (signal) for "10". In Sinuous/others you can't write to a computed because they're special.

☝ Too many dollar signs πŸ’Έ

But Dipole constantly writing ".get()" every time is not too much? πŸ˜‰ I picked a symbol because I want it to read as English and doing data.count(sub) or data.count(s) (which actually I used to use s back in the day) reads as the plural "data counts"? Isn't that weird? I'm open to other symbols though, let me know if you find something that works well.

... ugh - manually passing that same argument again and again everywhere.

You said yourself that you only write code once and read it a ton more.

Did you consider maybe something like this instead?

Yeah I did and didn't like it. I went with making "$" an unpacker function (with type safety too!)

const [count, text] = $(data.count, data.text);

☝ I know that's actually the same operation as the other reaction... I know Sinuous does something similar

Ok I'm glad that clicked! Yeah I agree it's a bit of magic. I'll write about it in an intro. Yes I totally stole that from Sinuous haha. Again, I want to be explicit but I want it to be short enough to have a good developer experience... I hope it's still more explicit than Sinuous' inline ${count} magic syntax. Also, in the screenshots above, you'll see "wR:1{wS:2{count}}" and that naming only happens when using the shorthand syntax, since using $ => ... is an anonymous function so the reactor doesn't get a name. So with the shorthand notation you can tell at a glance in the console that the reactor's job is to simply print a signal to the page, nothing else.

Re: https://github.com/heyheyhello/haptic/issues/5#issuecomment-827397013

... you like grouping states into "container" like structures

I actually just like names and objects are the only way I know to get naming for free.

... functions become computed reactive expressions.

Ok but I want to allow storing functions into signals and have them be untouched. How do I do that? I'd have to pack/unpack it in an object/array to sidestep Haptic touching my function? That's why I picked reactors, since that's a data type I can have control over... I figured if I make wR explicit in JSX then I ought to make it explicit in signal definitions for the exact same reason of consistency.

nettybun commented 3 years ago

Part of the importance of naming is for debugging when doing a JSON stringify of a reactor

image

You can see the function body right next to the signal names and reactor names.

This actually isn't reproducible rn since I've paused the dev bundle work to work on other things. The last time I was serializing reactors was in https://github.com/heyheyhello/haptic/commit/9f16cec8cac138c96628decac0a458566c2d7efb#diff-e3953e774715849eafdab3fdea6846ca283fadf588237fcb4c019a028419e17b but I'll be back soon!

mindplay-dk commented 3 years ago

I've actually never mention trying to be a rewrite or replacement of Sinuous πŸ˜… That's a non-goal!

I'm sorry, I misquoted you! The README says "fork of Sinuous", and my assumption was wrong. 😊

(I might suggest something like "inspired by Sinuous" or "borrowing ideas from Sinuous", or maybe even leave Sinuous out of the description altogether, since this looks and feels quite different.)

They have proper names actually.

Oh, I know - I just wish you would give them more down-to-earth names. wireReactor is the most important, because you'll be using that everywhere, and it's the first thing people will be exposed to. I'm not wiring a nuclear reactor, that sounds terrifying. πŸ˜… ... I wish you would go with just reaction, like Dipole - a reaction is the thing we're trying to get out of it. Since you'll be typing this more than anything, it's also nice that this doesn't involve pressing the SHIFT key - typing wR takes me longer than typing reaction and typos are more likely. It also, as said, doesn't read well, since there's not an actual word you can recognize on the page. (I sort of wish $ could be described with a two or three letter word for the same reasons - it's clumsy to type and it looks noisy.)

I realize that's completely opinionated, and feel free to ignore these comments! But Sinuous, to me, personally, reads a lot better than the two-letter abbreviations and $ signs everywhere. I understand (and agree with) your arguments for the explicit call to create reactions, type-safety and all, but that in itself already adds a lot of parens and dollars-signs, making it harder to parse and read. Using proper words decreases the per-character density of symbols, which (for most people, perhaps other than dyslexics) makes it overall easier to parse and read.

Yeah again I need to intro what's happening here. Thanks for calling it out! So there's actually a super important reason for packing it as a single object; signals/reactors have names! 😁 Like real names! They appear in stack traces and make debugging super easy in the console.

I can see that - I actually did something similar in an experimental state management library for React a couple of years back, but ultimately abandoned the idea. For one, it forced a grouping of potentially unrelated dependencies - you loose the "atomicity" of individual observables, you have to work with these state "molecules" instead. It wasn't practical when your component has only a single state value.

Presumably, with your implementation, you can still "pull off" one of the individual observables and pass it around though? They're not coupled to each other per se, right? You're just using the object syntax as a means of ensuring the observables have a name in the source-code that correlates with a name at run-time? I definitely don't hate that. 😁

It's completely individual how you'd weigh the pros and cons here, so I completely respect if that's the way you want to go. πŸ™‚

I get that you like const count = observable(0) but due to JS evaluation order there's no way to know the name unless you do count count = o(0, 'count') which I don't think anyone will want to do.

I think it would be nice to have the option though, since that's actually what it's doing at a low level, right? I mean, for a component with only a single state value, this isn't any better:

let count = wS({ count: 0 }).count; // count, count, count!

At least with let count = o(0, 'count') you're only repeating yourself twice.

Also, maybe the name could be optional, since it's not required for anything other than debugging? All these names will add to the overall source footprint of an application, and the repeated data. dereferencing also contributes to footprint and causes a minimal run-time overhead - so it might be nice to have the option to go the Sinuous route and forego the extra debugging convenience in favor of "smallness". It's only "free" in terms of effort - names always leave a source footprint.

But Dipole constantly writing ".get()" every time is not too much? πŸ˜‰

Well, but it's a word - I can skim across the page and recognize the word "get" easily. With something like Sinuous, it's much harder to recognize () in a sea of other parens and other symbols. Words are just inherently (for most people) easier to parse than symbols, much less pairs of symbols, which requires you to parse as well...

You said yourself that you only write code once and read it a ton more.

...yes, but words πŸ˜„

I want to allow storing functions into signals and have them be untouched. How do I do that?

In situations like this, I like to use an explicit "value cast", e.g. value(() => { ... }) - this would return a boxed value, so the container knows you want the actual function and not a reaction.

To me, it's a question of weighing the most likely use-case. How often do you really want to put a function in an observable? Seems like this would be pretty rare, compared with how often you'd want to compute something.

It's not an incredibly important point for me though - if you want to stick with object properties as a means of establishing logical names, I'd be fine with something like this as well:

const data = wire({
  text: observable(''),
  count: observable(0),
  countPlusOne: reaction($ => data.count($) + 1),
  countPlusTwo: reaction($ => data.countPlusOne($) + 1),
});

The main point for me is use words - make it easy to spot all the observables and reactions being wired together. πŸ™‚

(By the way, $ => data.count($) + 1 should work fine without the explicit return-type, right? The return-type can be inferred?)

nettybun commented 3 years ago

I've been thinking about your comments on the naming of exports and on anonymous signals. Sorry to leave you hanging, I'm moving houses right now. I see where you're coming from - I'm not sure I agree, but I tried it in a new branch at https://github.com/heyheyhello/haptic/tree/haptic-wire-rename. It adds 21 bytes (min+gzip) to the bundle. Here's a compare https://github.com/heyheyhello/haptic/compare/haptic-wire-rename

These are the exports:

export {
  signal,
  signalObject,
  sub,
  subClear,
  subPause,
  subAdopt,
  transaction,
  v$
};

Naming is really hard. I spent a ton of time in a thesaurus haha. The trick isn't just readability and familiarity but also that I want wS/wR to come across as a pair - they're not useful on their own, only together. I was hoping the shared prefix and two-letter short form indicated that. Maybe "dataWire" and "subWire". I'll never pick a perfect name...

... you have to work with these state "molecules" instead

Yes like Flux/Redux state stores. It's popular and my coworkers love it... For single state values you can use wS/wR like:

function SimpleComp() {
  const local = wS({ count: 0 });
  return <p>....{wR(local.count)}...</p>
}

Even tighter you can do const { count } = wS({ count: 0 }); and that's only two uses of "count", not three. It's also type safe since you have to unpack as "count" but with signal(0, 'count') you're open to making a typo in the naming and never seeing it in code since the name is internal.

I'd also argue that it's harder for a JS minifier to remove the name for signal(0, 'name') because it's a function parameter - minifiers don't want to touch that. Meanwhile in React it's the default to use state/setState with an object and minifiers are OK mangling object keys. We use this at work all the time to obfuscate the code.

you can still "pull off" one of the individual observables and pass it around though? They're not coupled to each other per se, right?

Yeah not coupled at all, but if you make a batch/molecule then all the signals have the same ID number, since object key names are already guaranteed to be unique. So in debugging if you see wS:3{text} and wS:3{diffCalc} you know they were created together.

... dereferencing run-time overhead

Sorry but I'm not interested in debating the performance run-time overhead of an object lookup... I think effort is the important thing to offer for "free"; everything else we have tools for like minifiers and source maps.

I can skim across the page and recognize the word "get" easily

image

I like that functions are bold in most colour themes. I get that not everyone has colour themes like that, but I think the bold stands out enough that using "get" makes the actual signal name fade into the background.

I'd be fine with something like this as well

Function names are immutable, so I'd have to wrap them, which adds to the size of every stack trace... I'll think about it more :)

By the way, $ => data.count($) + 1 should work fine without the explicit return-type, right?

I wish but no. It's a limitation of the TS compiler. I picked the best of many bad options as seen in the TypeScript Playground link at https://github.com/microsoft/TypeScript/issues/43683

mindplay-dk commented 3 years ago

I like these names better, though I still with it sounded less like technical jargon and more like everyday words - for example, I really like Dipole's "action" and "reaction", and I think part of the reason React succeeded with hooks is by "dumbing it down" to mostly plain english phrases like "use state" and "use effect".

IMO there's no shame in naming things naively, as opposed to being technically correct. Ordinary people have to build ordinary things on top of these core concepts - when things already have ordinary names, it encourages people to do the same, which leads to a more inclusive community.

Naming things is hard. My own personal struggle is not usually to find the correct computer science terminology - but rather to find plain english words that help people relate to the concepts. 😊

Even tighter you can do const { count } = wS({ count: 0 }); and that's only two uses of "count", not three.

Nice, I hadn't thought of that. πŸ™‚πŸ‘

I like that functions are bold in most colour themes.

This is in deed a good point! I had only been viewing/editing in the <textarea> here, so I hadn't thought of that. Forget "get" 😁

By the way, $ => data.count($) + 1 should work fine without the explicit return-type, right?

I wish but no. It's a limitation of the TS compiler.

Ugh. πŸ˜•

I remember having similar issues when attempting to write a type-safe IOC container for TypeScript a couple of years back, I also eventually gave up on that for the same reason.

Typing out redundant type-hints for every closure is going to be a big detractor for me.

It's pretty frustrating when you hit a complete dead-end in TypeScript - where you can describe exactly what it is you want, but TypeScript just won't let you do it. The inconsistency with double-nested closures is just straight up wonky. 🀨

Anyhow.

Thanks for the discussion, and feel free to close this issue. πŸ™‚

nettybun commented 3 years ago

Thankfully you never need to manually specify the type for signals or reactions, only when defining a computed-signal that uses signals defined in its own object/molecule. The TS issue is about "data" referencing itself in in its own initializer. So it's pretty rare to need to manually specify the type, and, oddly, if you specify the wrong type the compiler will error πŸ™ƒ which means it's pretty safe.

Some people like to have a separation of concerns too and might opt to storing computed-signals in a different object entirely, in which case there is no error.

I'll leave the issue for a bit while I write docs/intros and decide on naming. Lots to think about.

mindplay-dk commented 3 years ago

I'll leave the issue for a bit while I write docs/intros and decide on naming. Lots to think about.

I appreciate your considering my input - I'm quite interested in seeing where this project goes! πŸ™‚

The TS issue is about "data" referencing itself in in its own initializer.

This was exactly the issue with my IOC container, it had the exact same type relationship - here's the question and answers on StackOverflow, sadly with no happy conclusion:

https://stackoverflow.com/questions/51176441/how-to-declare-generic-mapped-types-for-this-dependency-injection-container-in

I'm pretty sure I opened an issue in the TS repo as well, but I couldn't find it...

mindplay-dk commented 3 years ago

Wait a minute. If I'm required to call wR explicitly wherever I want to create a reaction, isn't that enough? πŸ€”

I mean, doesn't that mitigate the risk of accidental subscriptions in itself?

I guess I can see how explicitly passing the $ context also removes element of "magic" though.

But I might want to take one step first and see how that works out in practice.

I might try it out in my own toy Sinuous clone to see how that pans out. πŸ™‚

nettybun commented 3 years ago

Yeah no the $ token is important because while you're right that wR is explicit for a reaction, it's only explicit at the top level, like that function body only. So in other libraries calling doThing() inside your reaction doesn't tell you if it'll subscribe to things, but in wire you have to explicitly give consent to the function by giving it a valid $ token as a parameter (that's also where v$ comes from, if you don't want to give that consent)

Wire is opt-in while other libraries use sample() to opt-out. I didn't like that because the only safe way to call a function is to always wrap every function call in a sample()...

mindplay-dk commented 3 years ago

Yeah, hmm.

What if we implicitly created a reactive context at every component boundary?

I realize that would result in a lot of "unused" reactive contexts with no subscribers - but I've been wondering, in the reactivity layer, what if we had a function that only creates a context if the function actually subscribes to something? If the function doesn't subscribes, no harm, it just returns the value of the function call as it would anyhow, but no actual listener gets created.

That could work?

nettybun commented 3 years ago

It could work but why tho... I think passing a parameter is the most explicit and least magic way to do it.

I did a weird thing with arguments.caller in my haptic-reactivity repo (different repo) way back where I isolated by function call. Subscriptions were always limited to their function scope. For nested functions you had to choose to merge its subscriptions into the reaction (called "sFrom()") or explicitly ignore them ("sIgnore()")... Not sure if the code would be useful if you're trying to implement something similar with function boundaries.

I don't think I'd prefer it over what wire currently does

mindplay-dk commented 3 years ago

It could work but why tho...

Convenience.

I think passing a parameter is the most explicit and least magic way to do it.

Definitely.

Are there any practical uses of $ for anything other than passing it back to observables?

If it isn't of any practical use to the developer, I think it makes sense to hide it somehow. In that case, hiding it is probably less "magical" and more "abstraction", e.g. hiding an implementation detail.

Is there anything meaningful a developer can do with it, other than slavishly passing it along?

nettybun commented 3 years ago

Is there anything meaningful a developer can do with it?

Subscribe? Sorry if I'm not understanding. You pass it along so nested functions can subscribe.

There has to be a way to differentiate between a read-pass (neutral? read) and a read-subscribe. This code will update when count changes but not when text changes:

<p>Number is {wR($ => data.text().length + data.count($))}</p>

I'm not too sure how this could hidden unless you use sample() but I want wire to be opt-in not opt-out.

<p>Number is {wR(() => sample(() => data.text().length) + data.count())}</p>

I also have consistency checking so you can't do data.count() + data.count($) in one reactor. I forget where I got inspiration for this from, but I'm not the first library to do it.

nettybun commented 3 years ago

Also I know I haven't written an intro - busy moving/unpacking still - but this is also where void tokens come in:

import { thirdPartyLatLngRecalculation } from '@company/someOtherTeam';
//       ^ function($: SubToken): [number, number];
<p>Number is {wR($ => {
  thirdPartyLatLngRecalculation()
  // ^ Expected 1 arguments, but got 0. ts(2554) An argument for '$' was not provided.
  return data.myStuff($)
})}</p>

You can fix this by calling it as thirdPartyLatLngRecalculation($v). This will skip subscriptions and TS is ok with it.

They should have typed their function as function($ = v$): [...] but every developer has dealt with an npm package that has a bug needing a workaround...

image

nettybun commented 3 years ago

Thought about it a lot. Let me know how "core" feels!

mindplay-dk commented 3 years ago

I'd like to try it out, but you haven't published to npm in 6 months?

I tried to add it directly from https://github.com/heyheyhello/haptic/tarball/haptic-win codesandbox, but I don't think that's supported - I was able to install it from tarball locally with a real npm, but would prefer to work in a public sandbox so we can review and discuss the results together.

Publishing unstable work with tag next is pretty conventional, I think? πŸ™‚

EDIT: hm, no, I can't get it to work locally either - it installs, but the imports in Typescript aren't working. It isn't being built, so I manually built it from the node_modules folder, but Typescript still can't see it. I'm probably doing something wrong? I don't know how to get this to work. Please let me know when there's a published version I can install and test?

mindplay-dk commented 3 years ago

Let me know how "core" feels!

Still extremely confusing. πŸ˜…

I had to dig into the source-code to understand which was what, then into the source-code, the git history, and the deleted example... I finally arrived at something like this:

import { signal, core } from 'haptic/wire';

const value = signal("foo");

core($ => console.log("value:", value($)));

value("bar");

It doesn't work, but it doesn't error either. As said, TS isn't picking up the types, so I'm flying blind - but from reading the source-code, I think this looks right? I'm a couple of hours in, and I still don't have a working "hello world". I feel dumb now. πŸ™„

You did mention this earlier:

I went with making "$" an unpacker function

Did you? The type declarations don't seem to reflect that change though? Not sure, as I'm reading them manually, as said TS isn't picking them up... If you did change this, then reading a value now would be the reverse call, right? $(symbol) would return a Sinuous-style reader/writer?

core($ => console.log("value:", $(value)()));

That didn't work for me either - no function I pass to core ever seems to get called.

I noticed in a comment somewhere, it says "activate the core by manually running it", so I tried that as well:

const value = signal("foo");

core($ => console.log("value:", value($)))();
//                                        ^^ <- activate?

That does print undefined, at least - but not the signal's value. And doing something like value("bar") afterwards, trying to update the value, does not call the function again.

Well, I'm lost. Let me know when there's a working example, some documentation, or some tests I can refer to. 😊

mindplay-dk commented 3 years ago

I'm eager to know how this pattern would work in practice.

Since I can't get your code to work, today I tried changing dipole to explicitly pass in context, just to see how different that would be. For observables and computed values, this was actually straight forward. For reactions, not so much - it needs to track the parent reaction so it can dispose of nested reactions, and there's no obvious way to do that. Manually passing the parent reaction to nested reactions looks and feels weird.

I looked the old example, which you deleted, but it didn't have an example of nested reactions.

Do you have a plan for that, or do you have any idea what that would look like?

To be clear, I mean reactive expressions, like (in Sinuous) for example:

    ${() => currentTab() === "A"
      ? <FormA/>
      : <FormB/>)}

Where FormA and FormB contain two different forms (that you can toggle with currentTab) which have entirely different reactions - when switching between tabs, these forms would get created/destroyed, and their reactions would be to different observables. How would you go about passing down the reactive context that gets created if it's explicit?

    ${wire($ => currentTab($) === "A"
      ? <FormA/ context={$}>
      : <FormB/ context={$}>)}

Not pretty. 🀨

Or will this bit still be "magic" and tracked via global state in Haptic?

mindplay-dk commented 3 years ago

I hope my questions didn't make you quit? I'm still really interested in this idea. πŸ™‚

nettybun commented 3 years ago

Heyy no not at all; life is just busy. I started working fulltime in software and have university, partners, and friends on top of that. I seem to not be doing a lot of hobby programming lately 🌲🚡. I haven't forgotten about your comments or my projects! They're on a list! Just have a lot of lists :) That's definitely part of not having published a new version in a long time. Thanks for your patience

With the lil energy I do have I've been putting it towards https://github.com/heyheyhello/acorn-macros. I mentioned it a bit in #6 that "[Haptic's] blocked on porting styletakeout.macro from Babel to Acorn so I can have CSS-in-JS". It's close tho! It fully works sync but still need to fix it for async and then do tests, docs, publishing etc etc... Haptic will come soon 🌺

You've got me here now tho so lemme answer your posts!


Publishing unstable work with tag next is pretty conventional, I think? :slightly_smiling_face:

OH! That's an AMAZING IDEA. Thanks! I've actually never made use of that in any other projects but it absolutely takes the edge off of publishing.

It doesn't work, but it doesn't error either.

Yeah your code is "fine" πŸ˜…. Reminds me a bit of writing just <App/>; in a React entrypoint - it won't mount on the DOM or do anything, but it's there...

In Haptic, like Dipole, you need to activate a core to start things. When you do core($ => {...}) you're defining it but it has never run (and never read any signals either). You do:

const c = core($ => {...});
c(); // Start!
value("bar");

I'll be sure to document that very carefully! It's in a JSDoc comment right now too but I'll be more explicit.

$(symbol) would return a Sinuous-style reader/writer?

Um no it's actually just supposed to make unpacking less painful to write. In Sinuous I often wanted to read a bunch of signals, something like this in Haptic:

core($ => {
  const [a, b, c] = [data.a($), data.b($), data.c($)];
  // Do stuff with the now-read values
})

Which works. But I made $ work also as /** Allow $(...signals) to return an array of read values */ which lets you write this: (Note that a, b, c will have the correct data type in TypeScript).

const [a, b, c] = $(data.a, data.b, data.c);

Just a shorthand.

That does print undefined, at least - but not the signal's value.

Huh that's odd. I'll make sure there's a test case for that :)

Manually passing the parent reaction to nested reactions looks and feels weird. I looked the old example, which you deleted, but it didn't have an example of nested reactions. Do you have a plan for that, or do you have any idea what that would look like?

Yeah! I definitely don't recommend passing $ as a context no no no. Nested relationships occur naturally based on JS function execution - this is what happens in Sinuous too. When a core runs, it's set as the "currently active reaction" and if another core is already in that spot, they're linked as parent>child. When the a parent core is destroyed it takes down its nested cores with it. S.js and Sinuous call this "automatic memory management" and I comment that in my code as well.

As for your "Tab" example, I often do this in Haptic:

<div>
  {when(core(($) => data.count($) > 5 ? 'T' : 'F'), {
    T: () => <p>There have been more than 5 clicks</p>,
    F: () => <p>Current click count is {core(data.count)}</p>,
  })}
</div>

You could do your method of a basic core ternary without using when but then flipping the ternary branches back and forth would constantly destroy and recreate new DOM elements and nested cores. Lots of wasted work!

In when the elements and cores are lazily created and then cached in memory - no work. Plus the branches that are "off screen" (the hidden tabs, in your case) have their cores paused so no work happens in the background even if signals are still firing, those cores are frozen. Hopefully that makes sense.

Or will this bit still be "magic" and tracked via global state in Haptic?

Haha sure it's magic ✨. Every library that I've used for reference and inspiration has done nesting this way, so, I'm doomed to repeat it :)

mindplay-dk commented 3 years ago

In Haptic, like Dipole, you need to activate a core to start things.

I see, hmm, well... this is actually something I objected to about Dipole:

The thing that puzzles me is, what do you actually have, if you haven't called the run method yet?

You don't really have a reaction yet - if it hasn't run the function, it doesn't know the function's dependencies, and so it can't actually react to anything yet.

When is an "inactive" reaction useful? Or why isn't the reaction automatically run the first time?

In your case, it's a factory function rather than a constructor - but the question is the same: if it's just variables bound to a scope (a context that doesn't actually work yet) what use is it? When or why would you want that?

If the intention is to defer... something? I can't think of what, but that was the Dipole author's argument... if you don't want the function to be run yet for some reason, just defer it's creation - don't make it the consumer's responsibility to "finish running the constructor" for you.

If you want the expose an internal factory function that doesn't fully initialize, that's fine for special use-cases maybe - but I don't think it makes sense as a default? It's really surprising when you call a constructor or factory function, and it returns something that doesn't do the one thing you want it to do. I don't think I'd be the first or only person to get hung up on that.

(fwiw, S.js initializes by default, and I don't think that's ever been a problem.)

I definitely don't recommend passing $ as a context no no no. Nested relationships occur naturally based on JS function execution - this is what happens in Sinuous too. When a core runs, it's set as the "currently active reaction" and if another core is already in that spot, they're linked as parent>child.

When teaching Sinuous to new users, I've found this is a serious stumbling block for them though. They end up accidentally subscribing, in a child-component, to updates from a parent component, leading to unexpected updates - which, in some cases (for example, if the parent context is a reactive expression that toggles a component on/off, or toggles between one or more types of components) can lead to unexpected recreation of the child-component and, subsequently, loss of any state it had. This has been the most frustrating and time-consuming problem for new users of Sinuous, from what I've seen.

If they come from, basically, any other framework, they do not expect updates to propagate into child components - almost all mainstream frameworks have update boundaries at the component boundaries.

State is generally local to component instances, and... I realize we don't have component instances in Sinuous/Haptic, but we do have JSX-style <Component/> tags - and it's really hard for new users to think of these as "just function calls", when in practice the result is something similar to having component instances with local state.

I realize it's not implemented that way - that you could do e.g. {Component({...}, [...])} as a JSX expression and it would work just the same, but... since we have the JSX component-style syntax, conceptually, it's natural for users to think of components in JSX as "component instances". (It would be surprising if they didn't.)

I think we have to let our brains soak on this for a bit? πŸ§ πŸ’­

If we want this type of framework to really catch on, it has to work without any major stumbling blocks. And I'd like it if we didn't resort to things like linters and run-time error messages like hooks for React. 😝

Ergonomics and "early success" is important. There's gotta be some way we can fix that. πŸ€”

nettybun commented 3 years ago

I know you've objected to it lol, I was in that thread and mentioned this library, Haptic Wire. I understand your frustration with it but I still like the separation of concerns. I'm sure there's a lot of people who'd love if React would just automatically mount the entry-level component so they could skip the "useless" React.render() call ;) In the Dipole thread you linked I also made a comment about vanilla DOM nodes - I think I'm mostly trying to be consistent the ecosystem around me.

As an analogy, create a core is me giving you a computer. It's turned off. I tell you there's a power button to turn it on when you're ready. This is useful if you need to install other components like a GPU which are more difficult to do once it's running. It has a nice computer-shape that feels like a computer (x instanceof Reaction === true) with all the ports you need (TS object shape is valid) so it can be given to functions that are specific/fragile and require a computer. If I put the computer in a box to defer it then these features are lost. There's no way to access it until it's too late and already running. "It's fine if it's already running by the time I get it" you say, well....

The most common thing people want to with reactivity is wire it to the DOM. Like <input value={core($ => someSignal($).toString())}. Two big things here:

(Now that I've said that out loud I actually think your wire($ => ...) naming might be more clear since I'm confusingly using "core" to refer to both an instance and a factory function...anyway...)

So, if core(...) produced a core that ran automatically then that <input attribute would receive the string content. Not a core. Which is a problem because Haptic DOM never had a chance to actually see and feel the core to install that GPU :) Problem. If I make cores auto-run I can't inline them.

Let's say I somehow overcome this - maybe I change it so calling a core returns itself kind of like how D3 does chaining. However, now calling the component that has the <input will 1) Execute the core, sending the computation result into the void and returning itself, then 2) Be patched by Haptic DOM, which now needs to call it again and do the computation again, which, if the core author had any kind of counter-like logic would be confusing and off by one.

Lastly, if someone needed to get the computation result but the core only returns itself, you either patch the c.fn like Haptic DOM does or you author it like core($ => { return (x = data.sig($)); }...

πŸ€·β€β™€οΈ


* Unless it's a computed-signal. Then it does lazy evaluation of a core. https://github.com/heyheyhello/haptic/pull/1. This is basically my version of Sinuous' computed(), except it's a signal. I always thought it was a bit backwards that Sinuous' subscription function, by definition, doesn't need/use any caching layer and yet it's implemented under the hood using a computed - it just ignores the whole variable-store mechanism altogether and only uses it for its auto-run functionality... I didn't like that

mindplay-dk commented 3 years ago

I honestly can't cope with all those details. πŸ˜…

Just. Did you see this note?

if you want to defer the initialization of something, just don't create it yet

But maybe the problem here is I'm creating the instance in my code - and then the framework is supposed to further initialize before activating the instance?

It still seems awfully surprising. Getting an instance that doesn't work. And the car analogy really doesn't work for me. It would be more like receiving a brand new car, but you have to open the hood and connect some wires before you can drive it for the first time. The way I was taught programming, objects should have certain guarantees - they should be born in a complete, working state. If you ship a car with missing parts or disconnected wires, well... no one would be very happy with the car company. πŸ˜‰

Anyhow, if the reason is the shared responsibility between client code and framework code for actually building and initializing the instance, then, yeah, there might not be any way around it. I don't have to like it though. πŸ˜†

mindplay-dk commented 3 years ago

(Unless maybe the revealing constructor pattern could help?)

mindplay-dk commented 3 years ago

I gave it another try and still can't get it to do anything.

const value = signal("foo");

core($ => console.log("value:", value($)))(); // prints `value: undefined` 🀨
//                                        ^^ core activation πŸš€

value("bar"); // does nothing πŸ€·β€β™‚οΈ
nettybun commented 3 years ago

Like I said I haven't touched Haptic in a while; it's not published because it's known to be in a half-baked state. I have other priorities and have been working on acorn-macros https://github.com/heyheyhello/acorn-macros/commit/e30a758c591d0b2ec79ae16bf998e796e515a6ac.

When I publish a new version of Haptic (even under haptic@next) you'll be the first person to know! :)

The issue you're experiencing is from a bad //@ts-ignore made a variable slip through the renaming from "Reactor" to "Core". You can change line 199 in wire.ts from && args[0].wR)) { into && args[0].wC)) { and you'll be good to go. I pushed the fix in 163ec1785f3c003f63a6758f6b4546e47567ed35 and will make note to use less ts-ignores... which is hard sometimes because TS has real bugs I need to workaround πŸ™ƒ

mindplay-dk commented 3 years ago

It's alive! πŸ˜„

Thanks, this gives me something to play with until you're ready to return to this project. I have a half-baked Sinuous clone based on Dipole that already mostly works, and I want to try to replace Dipole with this, just to see what it feels like in practice.

I will post my results, maybe there's something useful there to help this project move along. πŸ™‚

nettybun commented 3 years ago

I'm going to close this issue - I can't promise the syntax isn't cryptic but it sure has changed a lot since we started this thread. Good talk! Lots of ideas from here made it into v0.10. Thanks! When more syntax/cryptic questions come up we can start in a new issue or discussion. Plus you can message me on Discord, it's in my profile.