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

Determine strategy for making Forgo extensible without expanding framework core #54

Closed
spiffytech opened this issue Apr 20, 2022 · 7 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@spiffytech
Copy link
Member

spiffytech commented Apr 20, 2022

There will be bountiful cases where an app wants some extended version of what Forgo does today, but since Forgo aims to be simple and small, we want to arrive at a stable core that doesn't need constant tinkering and expansion to support new use cases.

What's the best way to facilitate this?

forgo-state is a good example of a framework extension. It's effective at its job, but I don't think it's a repeatable pattern. It has a lot of knowledge about Forgo's internals, which is both cumbersome and a barrier for non-core developers to do something similar. A hypothetical forgo-mobx library seems intense to implement right now.

Some examples of ideas requiring more than Forgo does today:

  • Automatically await callbacks and rerender when they return
  • Allow arbitrary functions to be run inside a component's error boundary, so that the UI updates appropriately if a click handler explodes
  • Hooks-esque stuff. Not really the way React does them (no magic variables, don't-call-it-stateful function components, etc), but e.g., if I use a setInterval(), I have to add mount + unmount methods and wire up both of those, plus the place I actually use the interval, touching three separate places of code. Right now it's not possible to make that setup/teardown reusable, short of creating a whole wrapped component. If args supported something akin to event listeners, that'd be easy for libraries to build on.
  • I've got a Portal component I need to contribute, but right now it depends on forgo-state which isn't ideal. It's easy to do without forgo-state, except I like that forgo-state minimizes what gets rerendered in a call tree. Making that logic reusable would be great.
  • How could an app be offered a Context API?

A few things I'm thinking through:

  • How can we let reusable code tinker with a component's lifecycle methods? What does that look like? If we support that, do we still need the lifecycle methods? Are we even willing to consider major API changes in the first place, even if the end result is still small and simple?
    • If wrapping components is still the way forward, how can we make that not boilerplate-y and error-prone? I'd guess a complex component might want 5-10 hooks-y things.
    • Maybe make component wrapping feel more like using middleware?
  • If some code wants to hook into attrs (e.g., to wrap click handlers), how would Forgo support that? Technically today you could just wrap each of your handlers manually, but what if you wanted that applied across your whole project? With arbitrary opt-outs?
  • How can we make forgo's core less of a leaky abstraction? Can we e.g., make userland blind to implementation details like args.element?
  • What other stuff like forgo-state's call tree evaluation makes sense to make reusable?
@spiffytech
Copy link
Member Author

I'm gonna stop saying "hooks-y", because I definitely don't want to bring React's style of hooks to Forgo.

Maybe a better term would be 'lifecycle event listeners', as distinct from the existing lifecycle methods.

I don't know what sort of API I'm thinking of here, I'm just kinda talking out loud as I think about handling the example use case.

@spiffytech spiffytech added question Further information is requested enhancement New feature or request labels Apr 21, 2022
@jeswin
Copy link
Member

jeswin commented Apr 22, 2022

Excellent ideas.

It has a lot of knowledge about Forgo's internals, which is both cumbersome and a barrier for non-core developers to do something similar. A hypothetical forgo-mobx library seems intense to implement right now.

Totally agree.

Automatically await callbacks and rerender when they return

I am thinking you mean something quite different from the following. Can you give an example?

function onclick() {
  await getSomeData();
  args.update();
}

Allow arbitrary functions to be run inside a component's error boundary, so that the UI updates appropriately if a click handler explodes

Good idea.

How could an app be offered a Context API?

React mostly uses it to inject state and state-modifying-functions (setters, reducers etc) into components. This is essential for React, because the essence of React is view = function(data) - and a change to data automatically should update the view. Forgo's view updates are manually triggered.

So, would it suffice to do a regular import (of a file such as the following) into the component?

// some-context.ts
export const myContext = {
  status: "active",
  secondsInactive: 0
}

How can we let reusable code tinker with a component's lifecycle methods? What does that look like? If we support that, do we still need the lifecycle methods? Are we even willing to consider major API changes in the first place, even if the end result is still small and simple?

Can you give an example of what this will look like?

Hooks-esque stuff...
I've got a Portal component I need to contribute, but right now it depends on forgo-state which isn't ideal...
... other ideas ...

I need to understand these a little more. Will comment this week.

@spiffytech
Copy link
Member Author

spiffytech commented Apr 22, 2022

Auto rerenders

I am thinking you mean something quite different from the following. Can you give an example?

This is something I'm lifting from Mithril's autoredraw system. The idea is that when a user clicks a button, you probably want to rerender. And if the button click triggers a network request, you probably want to wait for the request and then rerender*. If that's what you want nine times out of ten, it's nice if the framework (or an add-on library) does it for you.

What does it solve? 1) Reduced chance of bugs from some code path not including args.update(), 2) reduced boilerplate.

Put another way: Manual rerenders, though boilerplate-y, are better than the framework taking exclusive control of renders and trying to infer updates from state changes (like other frameworks). But I think inferring updates from DOM events is a good way to split the difference. I'm undecided on if it makes sense in core, but if not I'd like if Forgo could allow an add-on library to implement it.

On the verbosity front, let's take your example:

async function onclick() {
  await getSomeData();
  args.update();
}
return <button onclick={onclick}>Click me!</button>

With at Mithril-esque system, this would just look like:

return <button onclick={getSomeData}>Click me!</button>

Less code to read, less to go wrong.

This is a basic case. But if you throw in some conditionals, a try/catch, etc., then you amplify the chances that some code path won't correctly call args.update().

A user could implement this behavior today in Forgo:

// I guess this could just be async with `await cb()`, but this keeps synchronous stuff synchronous
function autoRedraw(cb, args) {
  return () => {
    try {
      const result = cb();
      // Test if result is a Promise
      if (Promise.resolve(result) === result) {
        result.finally(() => args.update());
      } else {
        args.update();
      }
    } catch {
      args.update();
    }
  };
}

...

return <button onclick={autoRedraw(getSomeData, args)}>Click me!</button>

The downside is this has to be applied by hand at every call site, for behavior you probably want across your whole app.

The alternative with today's Forgo is for getSomeData() to call args.update() itself, which at least simplifies declaring the click handler (no extra function declaration). Although it doesn't make manual updates any less error-prone. forgo-state can take on a portion of the auto-rerender part, but a) since it's not in core it doesn't feel like the default tool, and b) it won't do it when modifying mutable values, so it only offers partial coverage of the problem space.

Forgo could allow libraries to implement this if component had a method to modify attributes before they're put onto the DOM. Something like

{
  transformAttr(name, value, args) {
    if (name.startsWith('on')) {
      return autoRedraw(value, args);
    }
    return value;
  },
  render() {...}
}

Then you could imagine a library exposing this the same way forgo-state accepts a component and returns a component with modified lifecycle methods.

Pair this with wanting click handlers to run inside the component's error boundary and we've got two examples where modifying DOM node attributes is useful.

* Mithril was built before the Promise/A+ spec was ratified, so the only async stuff Mithril autoredraws for is its own built-in requests library


Contexts

How could an app be offered a Context API?

So, would it suffice to do a regular import (of a file such as the following) into the component?

This is how I'm handling it now (typically with forgo-state so I don't have to track what has to rerender), but static exports only work if the use case is amenable to a singleton. If you need something reusable (a repeatable component hierarchy, or a reusable application pattern), current Forgo requires prop drilling. Context APIs allow context to be created at arbitrary points in the component hierarchy, rather than as app globals.

I think it's also suboptimal to make state an app-wide singleton as a default. Sometimes that turns out to be a mistake, and then it's a whole big thing to change how it works. If the default is to make a Context, then if you suddenly need two independent copies of some component hierarchy, that's no problem.

The one big hassle with Contexts is types. A component expects to be run with certain Context values available, and I'm not sure how to express that in a type-safe way, guaranteeing a component is only instantiated in the correct context.

Example use case: in my app, I have a big list of cards with buttons on them. Click a button and refetch updated data from the server. I don't want the user to click buttons too fast, because when the network request finishes the layout in the affected region will shift, so whats under their thumb could change as they're reaching to press it.

So I want to disable all buttons in the affected region while the request is in flight, plus 500ms afterwards. There are 3-4 levels of component in between the top of the region and all of the buttons, so prop drilling the disabled state + the disable function would be a pain. And once I implement this, I'd like it to be reusable across parts of my app.

I don't want to make this a global singleton, because I want to gray out disabled buttons, and graying out the whole UI would be confusing, and also the user may want to go click things that have nothing to do with the affected region while the request is in-flight.

With Contexts I could make a MagicButton component that watches the context, replace all my stock <button>s, and nothing else has to change.

I don't think Forgo needs Contexts to come with magic auto-rerender behavior. Since rerendering an ancestor rerenders all descendants, descendants don't usually need to be reactive to Context state. And if they do, that's what forgo-state is for. It's just avoiding the prop drilling that's needed.


Runtime-modifiable lifecycle events

How can we let reusable code tinker with a component's lifecycle methods? What does that look like? If we support that, do we still need the lifecycle methods? Are we even willing to consider major API changes in the first place, even if the end result is still small and simple?

Can you give an example of what this will look like?

This is probably not a good API, but it at least demonstrates the idea I'm thinking of:

const MyComponent = () => {
  let socket;
  return {
    mount(_props, args) {
      const interval = setInterval(doStuff, 1_000);
      args.unmount(() => clearInterval(interval));

      socket = makeWebsocket();
      args.unmount(() => socket.close());
    },
    render() {...}
  }
}

The idea is that setup + teardown get put in the same place. Right now, it's easy to forget to implement teardown.

This starts to enable more reusable logic:

const MyComponent = () => {
  return {
    mount(_props, args) {
      useInterval(doStuff, 1_000, args);
    },
    render() {...}
  }
}

Now, intervals get torn down by default, no user discipline needed.

Questions here:

  • In this example, would we still want the unmount() component method? Or would we want just one way of doing things?
  • Is unmount the only thing this pattern is good for?
  • I think this could be implemented with a component wrapper today (at least for this contrived example). Is that the right decision?

If we take this pattern to its logical extreme, the component API may look like this:

const myComponent(_props, args) => {
  const {mount, unmount, afterRender, error} = args;
  return () => {
    return <p>Hello, world!</p>;
  }
}

I'm not necessarily saying the component API should look like this, I'm more putting it on the table to ask, just how open or closed are we to API changes in general? Are we considering Forgo's design "mostly done", or an evolving attempt to embody certain traits?

An alternative is we decide that wrapping components is the go-to way to go, and we make that super streamlined. That works for some things, like the interval where we want behavior but not data. But it doesn't work as well for the websocket example, where a component would want access to the socket to use in e.g., click handlers.

@jeswin
Copy link
Member

jeswin commented Apr 22, 2022

This is something I'm lifting from Mithril's autoredraw system.

Excellent examples and writeup.

I've always seen forgo's core as a tiny, lower-level lib with mostly explicitly triggered behavior. For example, that's why we have args.update(). But I'm interested in enabling all kinds of magic in libraries/wrappers that consume forgo, and your proposal to make forgo more extensible is essential in that regard.

I'm undecided on if it makes sense in core, but if not I'd like if Forgo could allow an add-on library to implement it.

I would strongly vote in favor of "Forgo could allow an add-on library to implement it". (As opposed to it being in core.)

Forgo could allow libraries to implement this if component had a method to modify attributes before they're put onto the DOM.

Good idea. And I don't think this will be difficult to implement. I can take a look at this, but you might have a better idea of what's needed. But do let me know if you want me to.

Contexts...

I agree that this could be quite useful, and your example is convincing. Not sure if there'd be a way to avoid prop drilling without contexts, but Context itself may not be hard to implement. We just need to decide on it. I am in favor of coming up with a proposal and then adding this feature; unless you think we can avoid prop drilling with some other technique.

Btw, can this (your example of disabling buttons) be done with forgo-state?

Runtime-modifiable lifecycle events

This is another interesting idea.

The question we need to ask ourselves is whether our events are multicast or not. As of now they're just a function - do we need them to be lists of functions? If we go that route, we'd also want a want to detach a function from an event, and also a way to list the functions currently attached to an event.

If we decide to go this route, we could assume that events can have a list of attached functions - and the current structure (which we already have) could form the first function in that list. We should also keep in mind that it may not make sense for render to be "multicast" - what are we going to do with two renders?

Alternatively, we could throw away the current structure entirely - and make component construction a run-time thing.

function MyComponent() {
  return (component: ComponentBuilder) => {
    component.mount(() => {
      const interval = setInterval(doStuff, 1_000);
      component.unmount(() => clearInterval(interval));
      const socket = makeWebsocket();
      component.unmount(() => socket.close());
    });
    component.render(() => <div>Hello world</div>);
  };
}

This wouldn't be too difficult to implement, because it doesn't touch the hard parts (bookkeeping) of forgo.

@spiffytech
Copy link
Member Author

Attr transformer

I can take a look at this, but you might have a better idea of what's needed. But do let me know if you want me to.

Go ahead and look at it. As I presently understand the problem, the important part is something that receives an attr name + value and returns the value Forgo will set on the node.

Watch out for idempotency (#32). Maybe we only want to call the transformer method if the inputs have changed? If we call it every render, it'll return a referentially-inequal modified value every render.

Contexts

I am in favor of coming up with a proposal and then adding this feature; unless you think we can avoid prop drilling with some other technique.

I can't think of an a way around it with the existing API. Everything comes down to, if you want an out-of-band way to do this, you still have to communicate which specific out-of-band instance (key, symbol, etc.) you want to reference. Which comes back down to prop drilling or asking the framework to do the drilling for you.

To toss out an alternative, instead of putting contexts into core, a library could implement the feature if forgo had an API to walk the component ancestry and a stable component identifier.

const contexts = new WeakMap<ChildNode,unknown>();
export function set(ref, key, value) {
  contexts.set(args, value);
}
export function lookup(args, key) {
  return contexts.get(args.element) ?? lookupContext(forgo.getParentRef(args, key))
}

...

function MyComponent(_props, args) {
  libcontext.set(args, 'foo', 123);
  const bar = libcontext.lookup(args, 'bar');
}

I don't like that this leaks args.element to userland. I'd prefer a stable opaque identifier like a Symbol. But then Forgo would have to bookkeep a Symbol<->ChildNode lookup.

I do like that removing implementation details from forgo-state already calls for an API to walk the component ancestry, and that this aligns with the vision of an extensible core, rather than putting another feature into core.

Btw, can this (your example of disabling buttons) be done with forgo-state?

Sorta. In the current form of my scenario, there are known, hardcoded regions that need to be managed, so I think forgo-state would work for the case in front of me (where Foo & Bar are each full of cards):

image

But if I wanted to independently affect dynamic regions it'd get trickier (where each Foo is still full of cards, but now there's a bunch of separate Foos):

image

forgo-state might do it using bindToStateProps, but only if the dynamic regions had obvious keys that were globally unique across my app. I'll bet a bindToStateProps approach would be okay-ish for many scenarios, but it amounts to the developer inventing an ad-hoc approximation of scoping rules which will break down in complex scenarios.

Lifecycle events

function MyComponent() {
  return (component: ComponentBuilder) => {
    component.mount(() => {
      const interval = setInterval(doStuff, 1_000);
      component.unmount(() => clearInterval(interval));
      const socket = makeWebsocket();
      component.unmount(() => socket.close());
    });
    component.render(() => <div>Hello world</div>);
  };
}

This style feels good to me. What do we get from nesting the arrow function within MyComponent?

This style starts to feel a bit like SolidJS, which might be a good direction to go. Their approach seems like a logical evolution of what major frameworks have turned into.

I like this style more than trying to add events on top of the existing monolith object API.

We might need to make some multicast and some not and eat that as a documentation issue. transformAttrs would be the same as render - no point in having two of them, you'd really want (manually?) composed functions instead.

@jeswin
Copy link
Member

jeswin commented Apr 23, 2022

These are all separate proposals - we could discuss them in their own threads.
I'll create them.

@spiffytech
Copy link
Member Author

I'm going to close this, since we've got the separate issues for each idea that came out of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants