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

RFC: Signals and Slots Hooks #135

Closed
wants to merge 6 commits into from
Closed

Conversation

Symbitic
Copy link

@Symbitic Symbitic commented Dec 4, 2019

Summary

This proposal is intended to introduce the concept of Signals and Slots used by Qt to React.

Signals and slots would be a natural, React-like way of enabling inter-component communication.
Simply passing messages between components shouldn't require passing callbacks that mutate
the parent's state or wrapping the Context API or a full-fledged state-management library like Redux.

It's backward compatible (signals can be connected even to old components).

Read the full RFC here.

Example

import React, { useSignal, useSlot, useState } from 'react';
import PropTypes from 'prop-types';

function TextInput () {
    const [ value, setValue ] = useState('');
    const emit = useSignal('updated');
    const onChange = (e) => {
        setValue(e.target.value);
        emit(e.target.value);
    };
    return <input value={value} onChange={onChange}
}

TextInput.signals = {
    updated: PropTypes.string
};

function Banner({ name }) {
    return <h1>Hello, {name}!</h1>;
}

Banner.propTypes = {
    name: PropTypes.string
};

function App () {
    const name = useSlot('updated');
    
    return (
        <>
            <Banner name={name} />
            <TextInput />
        </>
    );
}

First proposal for Signals and Slots.
@theKashey
Copy link

Look like this could be implemented in a user-space with ease. Just one context, one useState and one useCallback to create connected component.

@Symbitic
Copy link
Author

Symbitic commented Dec 6, 2019

Look like this could be implemented in a user-space with ease. Just one context, one useState and one useCallback to create connected component.

Would you mind posting a code example of that, preferably something similar to what I have above?

You are right that it could be implemented in user-space, but this seems easier and more natural to use. Context requires creating a provider and exporting the context so that other components can use it. It involves a lot of extra code, and it can be difficult to determine what the code is doing just by looking at it. Almost anyone could understand the example above.

@everdimension
Copy link

@Symbitic I think a recent RFC (and the user-land solutions mentioned in it) seem to be very suitable to solve this problem: #130

E.g. checkout https://github.com/pie6k/hooksy

It's not a one-to-one mapping to your proposal, but arguably a more natural way to achieve the same thing

@FireyFly
Copy link

FireyFly commented Dec 6, 2019

I'm not sure I entirely follow what

const MyButton = Button;
const MyBanner = Banner;
connect(MyButton, 'clicked', MyBanner, 'message');

would do. Surely the first two lines merely alias the Button and Banner components to new names, and the connect call would still pass in the original (presumably hypothetically imported from another module) components?

@Symbitic
Copy link
Author

Symbitic commented Dec 9, 2019

@FireyFly I don't know enough about the internals of React to say for sure. That was just meant to be an example of connecting to specific instances of components. If you have any alternate syntaxes feel free to share them.

@everdimension I checked hooky and all the others listed in that RFC. Unfortunately, I don't think any of them are substitutes for this. All of them are are for sharing state, which is arguably something that falls outside the scope of React. Signals and Slots are for allowing components to communicate with each other without being binding them. The only state for each component should be its own internal state. Having state be managed outside your scope (or managing another components state) inherently leads to tighter coupling.

Signals and Slots is meant to solve the problem of inter-component communication, not cross-component state management. If you feel differently about any of this feel free to explain why.

@mAAdhaTTah
Copy link
Contributor

mAAdhaTTah commented Dec 9, 2019

That was just meant to be an example of connecting to specific instances of components.

It doesn't really do that, because the reassignment still points to the same reference.

@Symbitic
Copy link
Author

Symbitic commented Dec 9, 2019

It doesn't actually do that, because the reassignment still points to the same reference.

I'll have to put some thought into that. For now, does it at least illustrate what I'm trying to do?

@mAAdhaTTah
Copy link
Contributor

Loosely, yes, altho as mentioned upthread, this sounds like a glorified event emitter which probably better exists in userland.

@Symbitic
Copy link
Author

Symbitic commented Dec 10, 2019

Loosely, yes, altho as mentioned upthread, this sounds like a glorified event emitter which probably better exists in userland.

Events are a natural part of the DOM, and by extension React.

Signals and Slots is an implementation of the Observer pattern, without needing to implement the observer (the event emitter). The main motivation for using this over an event emitter is that both the sending and receiving components would need to have knowledge of the emitter.

Lastly, useReducer was already implemented in user-space as Redux. Wasn't it turned into an official React Hook because it made using React easier? If the main question is 'does this add value by making React easier to use and easier to understand,' then I think the answer is yes.

My next syntax idea would be something like:

const buttonRef = useRef(null);
const bannerRef = useRef(null);
connect(buttonRef, 'clicked', bannerRef, 'message');
<Button ref={buttonRef} />
<Banner ref={bannerRef} />

I will update the text of the RFC soon.

Change the proposal to use refs from useRef() instead of aliases in the previous draft.
Also rewrite parts of documentation to hopefully make the purpose and use-case clearer.
@Symbitic
Copy link
Author

Draft 2 has been added. Replaces the aliasing syntax with refs defined by useRef() for connecting components. Also updates the weaknesses section with several items discussed here.

Example above has been updated to reflect the latest changes.

@j-f1
Copy link

j-f1 commented Jan 5, 2020

Can’t this be implemented with a useState hook?

import { useState } from 'react'

function Button({ name, emit }) {
  const onClick = () => emit(`Hello, ${name}!`)

  return <button onClick={onClick}>{name}</button>
}

function Banner({ message }) {
  return <h1>{message}</h1>
}

function App() {
  const [message, emit] = useState('')
  return (
    <>
      <MyBanner message={message} />
      <MyButton emit={emit} name="World" />
    </>
  )
}
TS version
import { useState } from 'react'

function Button({ name, emit }: { name: string, emit: (s: string) => void }) {
  const onClick = () => emit(`Hello, ${name}!`)

  return <button onClick={onClick}>{name}</button>
}

function Banner({ message }: { message: string }) {
  return <h1>{message}</h1>
}

function App() {
  const [message, emit] = useState('')
  return (
    <>
      <MyBanner message={message} />
      <MyButton emit={emit} name="World" />
    </>
  )
}

@Symbitic
Copy link
Author

@j-f1
For something simple like that, yes, but that isn't necessarily scalable for large numbers of components.

Plus, It's still 'passing a function to a component as a parameter,' which is exactly what I am trying to avoid. Whenever you pass functions to a React component as a param, it inherently couples the child to its parent, because the parent is defining the child's behavior. For example, if the parent forgets to pass the emit function to the Button component, then the child's behavior will be broken.

Signals and slots is meant to encourage separation of concerns.
Producers are concerned only with what they will produce, not who will consume them.
Consumers are only concerned with what they will consume, not where it comes from.
The parent simply connects a producer to a consumer.


I admit that plenty of people have found usable alternatives to this, but I'm not sure if they would be scalable enough to built an entire production-level application out of them.

The syntax now uses a `useSlot` hook to capture signals in the parent.
@Symbitic
Copy link
Author

Symbitic commented Apr 1, 2021

This is a major update. Instead of parents acting as an invisible mediator between two children, parents now capture the value of a signal emitted by a child which they can then pass to any other child. No more refs or keeping track of individual component instances.

You can think of this as adding the observer pattern to React. But instead of notifying observers by calling one of their methods, they notify components of changes by using React component props, which automatically update whenever a value is updated.

Most users have likely already implemented something very similar to the observer pattern by using e.g.:

import React, { useSignal, useSlot, useState } from 'react';
import PropTypes from 'prop-types';

function TextInput ({ value, setValue }) {
    const onChange = (e) => {
        setValue(e.target.value);
    };
    return <input value={value} onChange={onChange}
}

TextInput.signals = {
    value: PropTypes.string,
    setValue: PropTypes.func
};

function Banner({ name }) {
    return <h1>Hello, {name}!</h1>;
}

Banner.propTypes = {
    name: PropTypes.string
};

function App () {
    const [ name, setName ] = useState('');
    
    return (
        <>
            <Banner name={name} />
            <TextInput value={name} setValue={setName} />
        </>
    );
}

Most users will almost certainly do something like this at some point. The top-level App component provides a method for modifying the state to TextInput and then passes the updated value to Banner. The problem is that App exposes its own internal state and then provides a callback to modify the state to another component. That means App is tightly coupled to TextInput (plus, modifying the internal state of one component from inside another is just generally a bad idea).

Compare this with the example listed at the top of the thread. useState() has been moved to TextInput, which means it no longer has a way of modifying the App's internal state.

Once again, this is something that can be done in user-space, but it doesn't fix the underlying problem. Each component should be able to isolate its own state from any other components.

Another extra benefit: once the code for this is added to the React internals, it could be used to implement Error Boundaries as a hook. The observer pattern would work well for capturing an exception and then passing it up the stack until it reaches a handler.

@gaearon
Copy link
Member

gaearon commented Aug 18, 2021

In the spirit of #182, I'll try to provide some quick thoughts rather than an exhaustive review.

One thing that definitely can't work out is using strings for identifiers. We've learned that lesson with the legacy context API and don't want to repeat that mistake. In a larger app, strings always clash, so two components that are sufficiently far away (or just nested deeply) will at some point think that update means two different things. This is why the modern Context API forces you to actually import the context. This "ties" components together explicitly.

A minor nitpick is the Component.something = ... configuration style. We've been getting rid of these (e.g. propTypes is out of fashion now), and would rather not add more. Though this is relatively minor compared to the "big questions" of the design.

Most users have likely already implemented something very similar to the observer pattern by using e.g. [...]

Yes, that's the canonical way to do this in React.

The problem is that App exposes its own internal state and then provides a callback to modify the state to another component. That means App is tightly coupled to TextInput (plus, modifying the internal state of one component from inside another is just generally a bad idea).

I don't agree with this assessment. From TextInput's point of view, it just receives a function that requests change. You could name its prop requestChange or onChange instead of setValue to really drive that point. TextInput does not actually know that setValue will touch App's state, so I don't think it's fair to say that App exposes internal state to it, or that it's a bad idea. This is idiomatic React. Note how App can still change its state shape, or even move that state further app, or add some logic like <TextInput onChange={handleChange}> and then handleChange may decide not to call setValue, or transform it before calling, etc. So App is fully in control of what exactly it exposes.

I agree there's still some question of just how much decoupling we want to convey through naming and patterns. One pattern we've wanted to encourage is putting a reducer's dispatch function on the context. And then your child component does let dispatch = useContext(AppDispatch), and dispatch({ type: 'update', value: v }). But it's up to the component that provides the dispatch context (in our case, App) to handle this message and interpret this. I don't think this is really different from what this RFC is suggesting. Your variant is more concise, and I agree some more concise way to do this "dispatch context" pattern would be nice. I'm just not sure that what you're suggesting really solves all these cases. It seems to be a subset, and currently it being based around global strings makes it untenable. If you change that, I can take another look and give more feedback.

@mAAdhaTTah
Copy link
Contributor

@gaearon

We've been getting rid of these (e.g. propTypes is out of fashion now), and would rather not add more.

Somewhat tangential, but this is news to me. Is there an intention to deprecate propTypes and if so, replace it with what? We use propTypes extensively because we're not using TS/Flow and it's an extremely helpful tool, so I'm surprised to hear it's considered "out of fashion".

@gaearon
Copy link
Member

gaearon commented Aug 19, 2021

There’s no immediate plan or anything like this but we’ve been de-emphasising it over the years, and much of the usage in the community has been replaced by TS. At some point it’s possible that we’ll stop “checking” prop types in React and people who want to use them would add check(props, propTypes) calls to their components. We could make a codemod for that. The nice thing about this approach is that it’s not specific to components and you can use it anywhere. So if you’re into runtime type checking, you can do this explicitly. Anyway, that is tangential. It’s just that we try to avoid APIs that assign properties to functions.

@Symbitic
Copy link
Author

@gaearon

It’s just that we try to avoid APIs that assign properties to functions.

This part is easy to avoid. The TextInput.signals thing was optional. I just added that as a sort of prop type for type checking. If Function.propTypes is no longer used/needed, then neither is this.

One thing that definitely can't work out is using strings for identifiers.

I'll definitely need to put some more thought into this. With Qt C++, you pass in a method name (connect(m_class, &MyClass::methodName)). Since the whole point here is that we are using functions instead of classes, that won't work.

If a signal is an event, then it doesn't need to travel all the way up the stack. It is only visible to the direct parent (i.e. App will be able to connect to TextInput, but ParentApp will not).

I don't agree with this assessment. From TextInput's point of view, it just receives a function that requests change.

Then let me try another way of wording it: I want App to be able to receive information from TextInput without TextInput needing to execute a function that was passed as a parameter.


Here's a more condensed example of what I'm trying to do:

import React, { useSignal, useSlot, useState } from 'react';

function TextInput () {
    const [ value, setValue ] = useState('');
    const emit = useSignal('updated');
    const onChange = (e) => {
        setValue(e.target.value);
        emit(e.target.value);
    };
    return <input value={value} onChange={onChange} />
}

function App () {
    const name = useSlot('updated');
    
    return (
        <>
            <h1>Hello, {name}</h1>
            <TextInput />
        </>
    );
}

In this example, TextInput doesn't depend on anything. Not any props, not a function-as-a-parameter, not a Context. It just publishes a value that a parent may consume. To me, that is much more reusable.

@Symbitic
Copy link
Author

Closed in favor of my newer RFC: #246

@Symbitic Symbitic closed this Apr 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants