Skip to content
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

State utilities #799

Closed
NullVoxPopuli opened this issue Feb 24, 2022 · 12 comments
Closed

State utilities #799

NullVoxPopuli opened this issue Feb 24, 2022 · 12 comments

Comments

@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Feb 24, 2022

This may be a bit earlier, but I found this pattern a bit helpful, and I feel it'd be nifty to have something built in to the framework.

For example:

given a file located at app/helpers/state.ts

import { tracked } from '@glimmer/tracking';

import { Resource } from 'ember-resources';

export default class State extends Resource {
  @tracked value: unknown;

  update = (nextValue: unknown) => (this.value = nextValue);
}

I can make one off state thingers in components:

// app/componentns/shadowed.gjs
import state from 'limber/helpers/state';

const attachShadow = (element, setShadow) => {
  setShadow(element.attachShadow({ mode: 'open' }));
};

<template>
  {{#let (state) as |shadow|}}
    <div {{attachShadow shadow.update}}></div>

    {{#if shadow.value}}
      {{#in-element shadow.value}}
        <link rel="stylesheet" href="/assets/tailwind.css">
        <link rel="stylesheet" href="/assets/vendor.css">
        <link rel="stylesheet" href="/assets/app.css">

        {{yield}}
      {{/in-element}}
    {{/if}}
  {{/let}}
</template>

no need for class.

This could get hairy if people were to nest let blocks over and over -- maybe that could be a lint or something, like "hey, use a class, silly"

@SergeAstapov
Copy link
Contributor

@NullVoxPopuli not sure I follow (def missing something) why you need the state to extend from Resource.
Are you trying to implement when https://github.com/chriskrycho/ember-simple-track-helper does but with resources?

@NullVoxPopuli
Copy link
Contributor Author

NullVoxPopuli commented Feb 25, 2022

why you need the state to extend from Resource.

It's a utility that I'm used to using, and gives access to the whole instance (as opposed to class Helpers which are just the return value of compute)

Are you trying to implement

Not deliberately. 🤷

which are just the return value of compute)

which simple-track-helper got around via returning a vanilla class instance within the function-based helper 🙃

@chriskrycho
Copy link
Contributor

chriskrycho commented Feb 28, 2022

I wouldn't describe that as "getting around"; it's just normal JS. 😂 In general, I think the combination of function-based helpers via your merged RFC and normal class constructors (even using class static constructors as a helper!) gives you everything you need for this, right?

import { tracked } from '@glimmer/tracking';
import { pick } from 'ember-composable-helpers';

class SomeStateBucket {
  @tracked oneProp;
  @tracked another;

  constructor({ one, another }) {
    this.oneProp = one;
    this.another = another;
  }

  static of({ one, another }) {
    return new SomeStateBucket({ one, another });
  }

  updateOne = (newValue) => {
    this.oneProp = newValue;
  };

  flip = () => {
    this.another != this.another;
  }
}

<template>
  {{#let (SomeStateBucket.of one="abc" another=true) as |state|}}
    <label>
      oneProp:
      <input
        value={{state.oneProp}}
        {{on "change" (pick "event.target" state.updateOne)}}
      >
    </label>

    <button {{on "click" state.flip}}>
      switch to {{if state.another "false" "true"}}
    </button>
  {{/let}}
</template>

@NullVoxPopuli
Copy link
Contributor Author

ooo I like that way of using the plain function helpers!

@sukima
Copy link

sukima commented Mar 10, 2022

This is a fantastic idea. I know because I had the same idea myself. I made the following helper and have been using it in my code and I found it invaluable. I've shown it to a few developers and they all said it makes the template very easy to understand.

Although I can only attest to my own experience with the usefulness of this idea I wanted to share my version which you can use in apps now.

I would like to offer my most enthusiastic support for this feature in ember-core. Anything we can do to help evangelize the power and expressiveness of template focused code.

Implementation

import { Helper } from '@ember/component/helper';
import { tracked } from '@glimmer/tracking';

export class Trackable {
  @tracked value;
  set = (value) => (this.value = value);
  toggle = () => this.set(!this.value);

  get is() {
    return { [this.value]: true };
  }

  constructor(initialValue) {
    this.value = initialValue;
  }
}

export default helper(([value]) => new Trackable(value));

Usage

{{#let (trackable false) as |flag|}}
  <button type="button" {{on "click" flag.toggle}}>
    Toggle
  </button>
  <div data-open={{flag.value}}>…</div>
{{/let}}

Real use example

I wanted a component that if provided an action would follow Data Down Action Up, but if it not provided such an action it would track the value internally.

{{#let (trackable @value) as |token|}}
  {{#let (or @onUpdate token.set) as |updateAction|}}
    <fieldset ...attributes>
      {{#each @options as |option|}}
        <label>
          <input
            type="radio"
            name={{@name}}
            value={{option.value}}
            checked={{is-equal token.value option.value}}
            {{on "change" (fn updateAction option.value)}}
          >
          {{option.label}}
        </label>
      {{/each}}
    </fieldset>
  {{/let}}
{{/let}}

In this example if we pass in @onUpdate we are opting in to us managing the @value but if we don't then the component still works and still registers with the form.elements and new FormData().

@chriskrycho
Copy link
Contributor

What’s the value you perceive of “having it in core”? To my mind (speaking only for myself here, to be clear), the fact that folks can build, and have built, this in ways that fit their own needs, and that it’s easy to do and can be as small or large and as simple or sophisticated as need be, argues quite strongly against its “being in core” in some way. Especially because that dramatically slows the rate at which it can be iterated on and increases the maintenance burden for Ember itself. Now, that doesn’t necessarily mean “in core” (i.e. a helper shipped by default) is wrong, but for my part I would want to see a very strong argument for why it should be there rather than existing, possibly with several variations solving the problem with different moves in the tradeoff space, in the community.

@sukima
Copy link

sukima commented Mar 15, 2022

What’s the value you perceive of “having it in core”? To my mind (speaking only for myself here, to be clear), the fact that folks can build, and have built, this in ways that fit their own needs, and that it’s easy to do and can be as small or large and as simple or sophisticated as need be, argues quite strongly against its “being in core” in some way.

Oh oh! I can answer this. TL;DR What is available in core is not contested.

It has been my experience that introducing a helper like this results in others complaining that helpers are hard to understand and reason about.

However, the same individuals are very happy using helpers such as those in Glimmer, ember-truth-helpers, and ember-composable-helpers. Thus the conclusion is that if Ember core endorses something it is un-contested. If it is an individual contributor it is an uphill struggle to get reviewed.


More seriously though, the idea that Ember could offer base primitives falls under the same reasons why ember-truth-helpers are a good candidate to be assimilated into core.

I feel that something like this could add some very nice flexibility to components out of the box in that logic that depends on state can be directly seen in the template in the guides and from ember new without the discoverability issues that comes with addons. —IMHO of course.

@NullVoxPopuli
Copy link
Contributor Author

I strongly agree that Core needs a set of "stdlib"-type utilities (or a stdlib addon -- mostly just a single place for all the common utilities). With the move to embroider and modern packagers, we can have tree-shaking, and it's 0-cost to users who don't use the utilities, and it's only net benefit for those that do.

We can do this today with a meta addon, but for tree-shaking to be a possibility, we'd have 0 app-re-exports, which makes it a strict-mode-only type of library, which the RFC for first-class-component-templates (the syntax for using strict mode), was only recently merged.

So, it seems I've talked myself out of having a stdlib type utility for now 🙃

@chriskrycho
Copy link
Contributor

chriskrycho commented Mar 16, 2022

(Once again, speaking for myself and not for anyone else on the Framework or TS core teams)

I have two points to "respond" here, and then a follow-on sketch of a related idea.

1. In Core?

I personally think that the mentality that "in core == not contested" makes some sense—but its inverse, that "not in core == contestable" is really weird and indeed quite unhealthy. (I recognize, to be clear, that that isn't what you're saying: you're saying that many people do feel that way and that has an impact on you as someone recommending a given package.) That framing "assumes facts not in evidence" about non-Core-maintained packages, and I hope to see it change dramatically in the Ember community in the years ahead.

We should be evaluating packages on their own terms:

  • are they well-maintained?
  • are they well-tested?
  • do they have good security policies?
  • do they have good community policies?
  • are they reliable or do they have tons of unresolved bugs?
  • are they widely-used?
  • etc.

There are some Core-maintained packages which don't do well on some of those criteria—and plenty of non-Core-maintained packages which do stack up well on all of them!

This is only going to become more important as we move into a world where Embroider is the default and the difficulty of interop with the rest of the JS ecosystem drops to more or less nothing. That's fantastic, because it means we don't need Ember-specific wrappers or Ember-specific implementations of ideas from the rest of the front-end ecosystem. But it will also require teams to ask those questions for any given package; "is it Ember core?" just won't be helpful for thinking about lodash!

All that being said, I will add that we're working on the communication side of this on the Core Teams, because we recognize this pattern and definitely sympathize with the way it impacts people like you, trying to make a case for any given package's adoption.

2. Primitives

The thing suggested here isn't a primitive; it's a thing that is more or less trivial to build with the primitives. The primitives in question are: tracked state and helpers. Some things which aren't primitives are nonetheless valuable to have as a Core-maintained/first-party package, and others are not.

ember-truth-helpers is a good example of something that should be first-party, because it's arguably missing language features for the template layer. It's pretty rare for a language not to support equality or boolean comparisons, after all!

On the other hand, ember-composable-helpers is not, and I would be very surprised to ever see those land as first-party things! To the contrary: just as we're moving away from needing a first-party way of doing string utils or object utils—just use one of the many excellent JS libraries out there like lodash!—we would recommend the same with those.

Net, I think that we do not need this particular thing to be "in core". For the actual use cases today, you can already do one of two things:

  • write a helper of the sorts we've described here
  • introduce a backing class

And remember: backing classes are fine and good! They aren't really particularly different from the state buckets coming back from helpers shown a couple times in this thread, except that they're more convenient. Unlike with classic components, where they were heavy and had a lot of lifecycle hooks and tons of other legacy baggage around how they interacted with templates, Glimmer components are super lightweight and are mostly just convenient places to put component-local state!


A related idea

There is an interesting thing in this space that could resolve one of the tensions folks have felt about First-Class Component Templates, namely that they like having separate files for their component-local state and their templates. There's no inherent reason we can't do that with FCCTs—we're in control of how the authoring works, after all!

So one thing we could do here—and Caveat: this is in MASSIVE HAND WAVE territory and by no means a thing we are committing to do—might be something like this.

First: a template-only component, but where the template tag has a this= "attribute" on it, referring to an imported class:

// profile.ts
import { on } from '@ember/modifier';
import ProfileState from './profile-state.js';

export interface Signature {
  Args: {
    name: string;
    save: (newName: string) => void;
  }
}

<template this={{ProfileState}} type:signature={{Signature}}>
  <p>Current name: {{@name}}</p>

  <form {{on "submit" this.save}}>
    <label>
      New name:
      <input value={{this.name}} {{on "change" this.updateName}} />
    </label>
    <button type='submit'>Save</button>
  </form>
</template>

Then, we have a related module which supplies what historically would have been the "backing class". But notice that it doesn't actually need to be, and in fact is not, a "backing class": it is just a class with some tracked state, which we can use as the this context for the template:

// profile-state.ts
import { type Signature } from './profile.js';

export default class ProfileState {
  #args: Signature['Args'];

  @tracked newName;

  constructor(args: Signature['Args']) {
    this.#args = age;
  }

  get name() {
    return this.newName ?? this.#args.name;
  }

  updateName = (newName: string) => {
    this.newName = newName;
  };

  save = (event: SubmitEvent) => {
    event.preventDefault();
    this.#args.save(this.newName);
  }
}

Again, MASSIVE HANDWAVE TERRITORY.1 There are tons of things we'd have to figure out before we shipped anything like this, including the relevant syntax (is this= the right thing at all?) and a bunch of semantics (does ProfileState get recreated whenever args change, or do we just rely on args being autotracked, and either way what is its own lifecycle?)—and more than that, decide whether we even want to do something like this. THIS IS NOT EVEN A PROPOSAL YET. But it's an interesting idea, and to my eye at least seems more interesting than having a "state" helper built in—because it actually is a primitive, rather than something which can very straightforwardly be done in user-land (including just authoring it yourself!).

Footnotes

  1. and I personally would probably never do this: I would just put the class in the same file! But it could be nice for folks for whom it does seem important

@sukima
Copy link

sukima commented Mar 30, 2022

@chriskrycho I agree with what you said. 👏 well said.

I do want to find some clarity on a related topic though. You mentioned

The thing suggested here isn't a primitive; it's a thing that is more or less trivial to build with the primitives. The primitives in question are: tracked state and helpers.

And in principle this is also how my brain thinks. I do this. I find small utilities I can use to make my life easier as a developer. These utilities are built on primitives.

However, this does not appear to work in practice and I am desperate (so desperate I will negotiate actual 💵 to get an answer) to understand why. A simple example:

import { Helper } from '@ember/component/helper';
import { tracked } from '@glimmer/tracking';

export class TrackedState {
  @tracked value;
  set = (value) => (this.value = value);
  toggle = () => this.set(!this.value);
  get is() {
    return { [this.value]: true };
  }
  constructor(initialValue) {
    this.value = initialValue;
  }
}

export default helper(([value]) => new TrackedState(value));

That helper is < 10 lines of code and yet despite its utility (as demonstrated by this very RFC) its reception has been anything but. With the basic theme being that a backing class is better then a template only helper.

The contention there was that the mental model of opening a backing class, adding @tracked and an @action setTracked() was easier to understand then {{#let (tracked-state) as |…|}} in the template.

I can't tell if this was a JavaScript vs. HBS thing or what but it was yet another highlight that if an idea comes from Ember it is uncontested.


I'm not defending anything here I only wanted to highlight that even the simplest of cases (i.e. helper(([a, b]) => a === b)) is still a point of some friction in the communities I've worked with. Again I have very little theory as to why this is.

What I wouldn't give to have some forum to discuss theories on this phenomenon as it is something I have found myself struggle with since my upgrade to Senior a few years ago.

@wagenet
Copy link
Member

wagenet commented Jul 23, 2022

There is some very good discussion here, but it doesn’t appear that any of it is on a path towards RFC. If someone would like to change that, let me know!

@wagenet wagenet closed this as completed Jul 23, 2022
@NullVoxPopuli
Copy link
Contributor Author

We kinda already have an RFC: #812

That, and eventually with Starbeam, we'll have the spirit of this issue / thread

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants