Giter VIP home page Giter VIP logo

Comments (14)

mindplay-dk avatar mindplay-dk commented on May 13, 2024 2

Agreed but who really wants to be writing getCount everywhere. It affects readability as the file sizes grows and less words means less things to visually parse.

Honestly, I go back and forth on this.

But imagine you have 50 lines of JSX and somewhere in the middle you see this:

<div>Total Price: {price}</div>

How quickly can you parse this and spot the bug? In many cases, a variable named price or count will contain the price or count, and this will be totally valid - in fact, in all other contexts besides JSX, that's most likely what things will look like.

While, yes, some reactive frameworks will actually detect that price is reactive, and invoke it for you, I don't like that feature, as it invites more inconsistency with expressions like {() => price() * taxRate} where you can't omit the call. (Incidentally, this example shows how confusing this can get - if taxRate is just a constant, {() => getPrice() * taxRate} is much less likely to invite an error by missing a call: {() => price * taxRate} looks completely correct.)

And sure, TS might catch these, but the issue is human readability.

Now picture this amid 50 lines of JSX:

<div>Total Price: {getPrice}</div>

This would immediately stand out to most people, I think? Anything named get is conventionally a function, which in a context like this needs to be called. You would most likely never even make this mistake in the first place.

The getter is a getter, so some kind of convention is required at least.

Even if it's this, that's still better:

<div>Total Price: {$price}</div>

At least something about this variable is indicating it's not actually the price.

So maybe this convention is a nice middle ground?

const [$count, setCount] = observable(0);

Again though, what you take the $ prefix to mean is likely a personal matter. I use it very broadly to mean "one call removed from the thing it's naming", so it actually fits the convention I like to use. For example, $menu might be a function () => document.querySelector("#menu") so it's "one call removed" from actually being the menu. Memoization, service locators, lazy factories, many things fits this "one call removed" convention, and even at times I'll have $$something for things that are two calls removed for some reason. (such as this IOC pattern.)

Honestly most people will find it easier to parse the word "get" than a symbol like $ - we're taught from childhood, and most of us automatically parse proper words without even thinking about it.

From my point of view, it becomes somewhat a question of whether you're optimizing for reading or writing - personally, I'd rather spend more time writing, if that makes code more readable, but I also understand that not everyone feels the way I do.

(Not saying I would hate the $ prefix for getters or anything - if we care about dyslexics and non-english speakers, the $ prefix might actually be quicker to parse for them, and it's probably not exactly a hinderance for the rest of us. Just good to weigh all the pros and cons, and know exactly why you've chosen a convention.)

Either way, thank you for engaging in this discussion! I'm really glad to have run into someone who seems to share so many of my ideas and opinions. 😄🙏

from signals.

mindplay-dk avatar mindplay-dk commented on May 13, 2024 1

Okay, so I actually tried the getCount / setCount pattern, and I have to take back that suggestion. It sounds nice in theory, but in practice, some functions, such as the map function (as someone requested in #3) will need the observable instance itself, so it can call it... So I was wrong, these are not two independent functions - the observable itself is a "thing" required (if rarely) by certain other things. I hadn't noticed, but an observable actually has another "method", the next function, so in that sense, it is an object with an identity and methods. (although that's not technically/precisely how it's implemented.)

As for my little Sinuous clone, I've added some more log statements to the first version here:

https://codesandbox.io/s/reactive-dom-maverick-njvhsw?file=/src/reactive-dom.js

It looks like the problem is nested effects, which appear to all fire at the same time? If you try pressing the "+" button on one of the counters, you can see every effect in every parent effect firing along with it. So I don't know how this was intended to work, but this example previously worked with S.js and dipole as well, so there's something substantially different about how this works, I guess? I've never fully grasped these libraries beyond using them, tbh, so I can't really help.

Do you plan to build a UI library based on this? Feel free to steal anything you can use from my crap. 😄

from signals.

mindplay-dk avatar mindplay-dk commented on May 13, 2024 1

How? I wish CSB had a source control interface. 😅

Either way, I think this issue is resolved.

from signals.

mihar-22 avatar mihar-22 commented on May 13, 2024

Hey @mindplay-dk!

I was able to quickly fix all of it except the <Counter /> component which I'll try to have a look again later. Other issues were simply that Maverick is updated via the $observable.set(...) function and not $observable(...). This is to clearly separate read/write actions, make it easier to parse code visually, and to allow functions to be observed such as callbacks.

Let me know if you find out what's wrong :)

https://codesandbox.io/s/reactive-dom-maverick-forked-5z0pt3?file=/src/index.jsx

from signals.

mindplay-dk avatar mindplay-dk commented on May 13, 2024

Other issues were simply that Maverick is updated via the $observable.set(...) function and not $observable(...)

Oh, whoops. 😁

How comes this appeared to be valid with the TS types though?

from signals.

mihar-22 avatar mihar-22 commented on May 13, 2024

It does but that's a .jsx file in CodeSandbox so no error diagnostics 🙂

image

from signals.

mindplay-dk avatar mindplay-dk commented on May 13, 2024

Oh gosh I'm tired today. 😅

from signals.

mindplay-dk avatar mindplay-dk commented on May 13, 2024

I was able to quickly fix all of it except the <Counter /> component which I'll try to have a look again later.

This is curious - it's issuing way too many updates... it looks like every property in every instance receives an update?

Line 89:

https://codesandbox.io/s/reactive-dom-maverick-forked-15t8q7?file=/src/reactive-dom.js

There may be something about contexts/roots that I'm not understanding. 🤔

This is to clearly separate read/write actions, make it easier to parse code visually, and to allow functions to be observed such as callbacks.

I think my confusion here stems from the fact that $foo() and $foo.set() have substantially different syntax. In most frameworks/libraries, this is a pair in some sense, usually with similar syntax - I get what you're trying to do, but it's as though this is following two different conventions, $foo() favoring brevity, and $foo.set() favoring clarity.

If you want to be really explicit, I would suggest the more verbose $foo.get() and $foo.set(). Although my typical quarrel with this style is the fact that this is not in fact one object with two methods, so it's kind of misleading.

I haven't seen any framework doing exactly this, but honestly, this would be my preferred format:

const [getCount, setCount] = $observable(0);

This makes all the client code completely explicit, and it accurately reflects what these functions do. I don't like the React hooks convention of naming the getter count, because that variable is not the count, it's a getter for the count, and it's just confusing and error prone to have them look like free standing variables, but then other variables that actually are free standing don't need a call, because they're not getters.

The $ prefix conventions for observables isn't very helpful either. The $ prefix has no conventional meaning anywhere - folks pretty much abuse this for whatever they think needs to "stand out" in the source code for some reason. Words like "get" and "set" imply actions, so you'll know they need to be invoked, and you won't be confused about count being a number or a getter.

I also don't know why the API itself uses $ prefixes? Those functions are not observables. So you have two different conventions both using a $ prefix.

Just my $0.02. The best convention is no convention. Just name things what they are. 😄

Anyhow, I will probably just wrap these functions and re-export.

from signals.

mihar-22 avatar mihar-22 commented on May 13, 2024

The $ prefix conventions for observables isn't very helpful either. The $ prefix has no conventional meaning anywhere - folks pretty much abuse this for whatever they think needs to "stand out" in the source code for some reason.

I decided to go ahead with this recommendation and drop $ from the library. Library authors can re-export as desired.

It looks like the problem is nested effects, which appear to all fire at the same time?

I made some important core fixes a little while ago so now that demo you showed me works perfectly:

https://codesandbox.io/s/reactive-dom-maverick-forked-8zxrn5?file=/package.json

$foo() favoring brevity, and $foo.set() favoring clarity.

This is exactly what I'm going for and I share the love/hate here too. I don't particularly like foo(someValue) as a setter as I want to clearly separate read/write actions which helps supports tracking functions, and I find const [getter, setter] = observable(); too verbose. I'm going to experiment with this API and based on additional feedback change accordingly. I completely understand what you mean. I'll share some more detailed thoughts on this once I put things into practice in an upcoming library I'm working on.

This makes all the client code completely explicit, and it accurately reflects what these functions do. I don't like the React hooks convention of naming the getter count, because that variable is not the count, it's a getter for the count, and it's just confusing and error prone to have them look like free standing variables, but then other variables that actually are free standing don't need a call, because they're not getters.

Agreed but who really wants to be writing getCount everywhere. It affects readability as the file sizes grows and less words means less things to visually parse. Getting a value is implicitly clear enough once you're aware of the pattern. To me there's no semantic difference, either way you're referencing some value and in this case it's a function which gets some value. As with all things I'll watch how the community evolves on these patterns and update my thoughts.

Thanks heaps for sharing all this feedback!

from signals.

mindplay-dk avatar mindplay-dk commented on May 13, 2024

Hmm, there is still a problem with my toy example - it appears to drop all prior effects when I create a new effect in the same context.

Here's a simplified example - everything looks fine at first, but as soon as you add a new counter, all the other counters become unresponsive.

I backported this example to the previous version based on dipole to assure myself this worked before. I went over the map code line by line, but I can't spot any significant difference - dipole doesn't immediately run effects ("reactions") so I had to manually return nodes when the effect initializes, but that's the only apparent difference.

So it just looks like a difference in behavior, whether intentional or not. I've had this example working with both s-js and dipole, so I'd expect it could work with your lib as well.

Again, don't feel obligated to look into this - it's just a toy project, I have no plans to publish a library. 😄

from signals.

mihar-22 avatar mihar-22 commented on May 13, 2024

Again though, what you take the $ prefix to mean is likely a personal matter.

It seems in JS user-land the $ sign is becoming a marker for "reactivity". Both Svelte (store subscriptions and computation labels) and Vue (ref sugar) have adopted this. There's so many terms that might fit what $ means in the modern context such as "reactive", "subscription", or a "stream". I had adopted this behaviour from said examples in the same way. Based on your feedback and some thought decided the best thing to do is let others determine the appropriate notations for their given library. As you said, sometimes "the best convention is no convention, just name things what they are."

To be more precise, the $ prefix in most libraries refers to a active subscription rather than a "special" function call. Unfortunately, the $ prefix is a little more vauge in functional libraries like this and S.js and I'm not sure how I feel about it yet. It's more of a lazy subscription that's created when you call an observable... so what does $ really mean here? I think it just helps identify some base primitive such as an observable and serves as a hint that this needs to be called. In terms of DX this is a small win and unforunately you can't do much without introducing a custom DSL like Svelte/Vue did. Compiler alone is not enough today. You'll need IDE integrations and the lot which is just a silly amount of work.

Hmm, there is still a problem with my toy example - it appears to drop all prior effects when I create a new effect in the same context.

I see. I thought the most sane thing to do is dispose of nested computations (here) when it's re-evaluated. Maybe it's not 🤔 I'll think about this and come back to it. I'm pretty sure it's why the old counters are dying, and the new are reactive until another one is added.

from signals.

mihar-22 avatar mihar-22 commented on May 13, 2024

Fixed 😄

https://codesandbox.io/s/reactive-dom-maverick-forked-meefbk

from signals.

mindplay-dk avatar mindplay-dk commented on May 13, 2024

Sorry for the late response, I somehow missed the notifications for your replies. 😅

I upgraded to the latest 4.3.2 release today, and it does look like everything works now!

Just for clarification, did you change anything in my sandbox? Or you just forked it so you could upgrade the dependency? (I only upgraded the dependency in my own sandbox, and everything appears to work.)

from signals.

mihar-22 avatar mihar-22 commented on May 13, 2024

No problem @mindplay-dk!

I'm pretty sure I only updated Maverick. Feel free to check my fork to see if there's any differences :)

from signals.

Related Issues (18)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.