Skip to content

Propose new reactivity utility: reactive from @ember/reactive #1079

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Mar 6, 2025

Propose new reactivity utility: reactive from @ember/reactive

Rendered

Summary

This pull request is proposing a new RFC.

To succeed, it will need to pass into the Exploring Stage, followed by the Accepted Stage.

A Proposed or Exploring RFC may also move to the Closed Stage if it is withdrawn by the author or if it is rejected by the Ember team. This requires an "FCP to Close" period.

An FCP is required before merging this PR to advance to Accepted.

Upon merging this PR, automation will open a draft PR for this RFC to move to the Ready for Released Stage.

Exploring Stage Description

This stage is entered when the Ember team believes the concept described in the RFC should be pursued, but the RFC may still need some more work, discussion, answers to open questions, and/or a champion before it can move to the next stage.

An RFC is moved into Exploring with consensus of the relevant teams. The relevant team expects to spend time helping to refine the proposal. The RFC remains a PR and will have an Exploring label applied.

An Exploring RFC that is successfully completed can move to Accepted with an FCP is required as in the existing process. It may also be moved to Closed with an FCP.

Accepted Stage Description

To move into the "accepted stage" the RFC must have complete prose and have successfully passed through an "FCP to Accept" period in which the community has weighed in and consensus has been achieved on the direction. The relevant teams believe that the proposal is well-specified and ready for implementation. The RFC has a champion within one of the relevant teams.

If there are unanswered questions, we have outlined them and expect that they will be answered before Ready for Release.

When the RFC is accepted, the PR will be merged, and automation will open a new PR to move the RFC to the Ready for Release stage. That PR should be used to track implementation progress and gain consensus to move to the next stage.

Checklist to move to Exploring

  • The team believes the concepts described in the RFC should be pursued.
  • The label S-Proposed is removed from the PR and the label S-Exploring is added.
  • The Ember team is willing to work on the proposal to get it to Accepted

Checklist to move to Accepted

  • This PR has had the Final Comment Period label has been added to start the FCP
  • The RFC is announced in #news-and-announcements in the Ember Discord.
  • The RFC has complete prose, is well-specified and ready for implementation.
    • All sections of the RFC are filled out.
    • Any unanswered questions are outlined and expected to be answered before Ready for Release.
    • "How we teach this?" is sufficiently filled out.
  • The RFC has a champion within one of the relevant teams.
  • The RFC has consensus after the FCP period.

@github-actions github-actions bot added the S-Proposed In the Proposed Stage label Mar 6, 2025
@NullVoxPopuli NullVoxPopuli mentioned this pull request Mar 7, 2025
11 tasks
@ef4
Copy link
Contributor

ef4 commented Mar 7, 2025

Notes from RFC meeting discussion:

The deep-tracking is under-specified and probably not viable except as a rarely-used curiosity. Consider:

let existingObject = { a: 1 };
let deepStuff = reactive({ });
deepStuff.b = existingObject;

if (deepStuff.b === existingObject) {
  // One would hope this is true!
  // But if it's true, how did `existingObject` gain tracking?
  // Did we mutate all its descriptors? How will we make newly-added properties reactive too?
}

Overall I am skeptical that this is any simpler than teaching people that there's a module from which they can import tracked versions of platform built-ins (Map, 'Object', 'Set') they already know. Those all give you controlled access to reactivity. I have yet to ever discover a use case for deep reactivity in a real app.

I think "import fatigue" can be mitigated by having a single logical package that exports them all. Then you only need to know the one package and that you can find tracked variants of the platform built-ins in there and that the tracked variants have exactly the same visible API as their normal counterparts. You aren't learning N different things, you're learning one thing and applying it to the N plain-javascript APIs you already know.

@JimSchofield
Copy link

From a user perspective, you'd gain common language. One strength with signals is that they're a first class entity in the frameworks they use them. In Ember, the notion of a "reactive primitive" is kind of ambiguous- there's making a tracked property on a class, making tracked set/map, but also making a cell. So there seems to be multiple "reactive" things floating around, and it would benefit using the same vocab.

So about this:

const data = reactive({ greeting: 'hi' });

What happens when a user wants to have a primitive?

const thing = reactive("some primitive");

I understand you can't do this with proxies, but you could with a cell. This leads to split implementations of reactivity that users need to think about- how you handle properties on classes and how you handle object VS how you handle primitives. Do you just enforce the face that if you need a tracked primitive you always wrap it in an object?

Re deep tracking: I would agree with @ef4. Taking care with deep reactivity is not uncommon in all other reactivity systems, and you can import a utility that handles it if you really need it. The only one that seems to handle it by default is Vue. In all the other reactivity systems- hooks, signals, svelte etc.- developers are used to shallow reference comparisons.

@NullVoxPopuli
Copy link
Contributor Author

NullVoxPopuli commented Mar 8, 2025

This leads to split implementations of reactivity that users need to think about- how you handle properties on classes and how you handle object VS how you handle primitives. Do you just enforce the face that if you need a tracked primitive you always wrap it in an object?

the ts playground example in the RFC shows how we can use types to guide users about which things they have to deal with (and cell is already included for primitive values).

Do you just enforce the face that if you need a tracked primitive you always wrap it in an object?

I think we do need to explain this this to users -- I don't think folks benefit from having information hidden from them (tho, progressive disclosure is certainly a helpful tool -- in order for folks "to get it", they need to have more information than we've historically given them so far -- I think other reactive ecosystems do a better job about teaching the mental model than we do)1. Reactivity, auto-tracking, etc, isn't magic. You need something to "hold" the value that we're making reactive. In classes, that's the this (ish), and in non-primitives, it's a proxy.


To summarize:

Questions Answered

What happens when a user wants to have a primitive?

const thing = reactive("some primitive");

This must return a Cell in order to be reactive, see: RFC 1071

While I have a way to describe the returned shape of thing in typescript, I'm hopeful that if we ship our ts configs, glint, etc in javascript applications, then folks will get the same benefit.

=== equality?

let existingObject = { a: 1 };
let deepStuff = reactive({ });
deepStuff.b = existingObject;

if (deepStuff.b === existingObject) {
  // One would hope this is true!
  // But if it's true, how did `existingObject` gain tracking?
  // Did we mutate all its descriptors? How will we make newly-added properties reactive too?
}

I don't think it's possible to have === equality without mutation.
And I don't want to encourage the idea of a mutation happening on the right hand side of an assignment. feels bonkers.

With the implementation I was thinking of (no mutation), here is how the scenario interacts:

  test('=== after assignment of object', async function (assert) {
    let existingObject = { a: 1 };
    let deepStuff = tracked({});
    deepStuff.b = existingObject;

    assert.notStrictEqual(deepStuff.b, existingObject);

    await render(<template>
      <div id="one">{{deepStuff.b.a}}</div>
      <div id="two">{{existingObject.a}}</div>
    </template>);

    assert.dom('#one').hasText('1');
    assert.dom('#two').hasText('1');

    existingObject.a = 2;
    await settled();

    assert.dom('#one').hasText('1');
    assert.dom('#two').hasText('1');

    deepStuff.b.a = 2;
    await settled();

    assert.dom('#one').hasText('2');
    assert.dom('#two').hasText('1');
  });

Started tracking stuff like this here: NullVoxPopuli/ember-deep-tracked#394

Questions Pending

did I miss anything? (anyone can answer these by the way -- they need answers before RFC can progress (towards closing or towards acceptance))

  1. shallow-reactivity is a good default, but the ergo of reactive.shallow feels like the framework would suggest we don't want folks using shallow reactivity. The RFC has been updated to have reactive() mean shallow, and reactive.deep() to be deep -- how do we teach against the troll that is this situation:
    let shallow = reactive({ a: { b: 2 } });
    shallow.a.b = 4 // not reactive
  2. with deep reactivity, is structuredClone fine?
  3. with deep reactivity, do we clone keys ever? (I think no? -- referential integrity retained -- so this is deep clone with caveats -- is this confusing?)
  4. Is there consensus on preferring tracked-built-ins built-in #1068 ?

Footnotes

  1. I think maybe our reactive teaching needs to be framed around what our goals are -- this isn't stated anywhere in the docs, RFCs, etc -- it comes up in comments here and there from various folks, but there hasn't been anything cohesive. Like, as a default we want folks to feel like they're using "just javascript" -- without having to do a bunch of custom APIs for things (such as what occurs when Cell (svelte signal, vue ref, or TC39 Signal, etc) is your core user-facing reactivity utility) -- this requires design tradeoffs -- for one, the existence of a "host object" (class or non-primitive/proxy), etc

@abeforgit
Copy link

abeforgit commented Mar 9, 2025

1. shallow-reactivity is a good default, but the ergo of `reactive.shallow` feels like the framework would suggest we don't want folks using shallow reactivity. The RFC has been updated to have `reactive()` mean shallow, and `reactive.deep()` to be deep -- how do we teach against the troll that is this situation:
   let shallow = reactive({ a: { b: 2 } });
   shallow.a.b = 4 // not reactive

one way to alleviate this could be to allow no default choice for compound types, e.g.:

const label = reactive('foo') //ok!
const label = reactive.shallow('foo') //ok! could be linted for consistency
const label = reactive.deep('foo') //debatable, could be the same as shallow, or could error 

const person = reactive({name: 'foo'}) // error! need to specify (maybe type/linter error only?)
const person = reactive.shallow({name: 'foo'}) //ok!
const person = reactive.deep({name: 'foo'}) //ok!

@gossi
Copy link

gossi commented Mar 10, 2025

I did some testing with how svelte behaves: playground

keep in mind, this is run through their compiler. Switching over to their js output is sometimes a bit closer to what we would write.

@NullVoxPopuli
Copy link
Contributor Author

do they have any answers for our questions?

otherwise, it looks like I need to explore and debug through this code: https://github.com/sveltejs/svelte/blob/main/packages/svelte/src/internal/client/proxy.js

@acorncom
Copy link
Contributor

acorncom commented Mar 30, 2025

One thought: the reactive.deep api reads quite nicely, but seemingly then requires the default export to include all deep tracking code to be included in all payloads? As an initial approach here, we may want to ship shallow by default, warn users that deep tracking an object isn’t supported at the moment (with links to options on how to achieve what they want) and consider paving that path later.

In general it seems good to teach folks to understand what is going on / encourage them to think more deeply about what they want to happen vs blithely helping them out (or attempting to with footguns later on)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-Proposed In the Proposed Stage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants