Proposal: presence #414
Proposal: presence #414
Conversation
|
|
Thank you very much @thetarnav for the detailed review! I've responded to all comments and for the sake of quick-scan reading all of my resonses contain either 👍, ❓, or 🛑 emojis. 👍 means that I agree with the feedback and have updated accordingly Client & server tests have be added for One leftover question I do have is what the situation is with adding a |
|
I believe implementation could be simplified significantly like so: import {
Accessor,
createComputed,
createSignal,
on,
untrack
} from 'solid-js';
interface Options {
initialTransition?: boolean;
duration: number | { enter: number; exit: number };
}
interface State {
get isMounted(): boolean;
get entering(): boolean;
get entered(): boolean;
get exiting(): boolean;
get exited(): boolean;
}
type Status = 'entering' | 'entered' | 'exiting' | 'exited';
export function usePresence(show: Accessor<boolean>, options: Options): State {
const { initialTransition, duration } = options;
const enter = typeof duration === 'number' ? duration : duration.enter;
const exit = typeof duration === 'number' ? duration : duration.exit;
const initialVal = untrack(show) ? 'entered' : 'exited';
const [state, setState] = createSignal<Status>(initialVal);
const handleTransition = () => {
const val = show();
setState(val ? 'entering' : 'exiting');
setTimeout(() => setState(val ? 'entered' : 'exited'), val ? enter : exit);
};
if (initialTransition) {
handleTransition();
}
createComputed(on(show, handleTransition, { defer: true }));
const getState: State = {
get isMounted() {
return state() !== 'exited';
},
get entering() {
return state() === 'entering';
},
get entered() {
return state() === 'entered';
},
get exiting() {
return state() === 'exiting';
},
get exited() {
return state() === 'exited';
},
};
return getState;
}Here state updates triggers single update, so we can avoid batching. The naming convention the react implementation uses a bit confusing, I think clear and straightforward names are better. You can see a working demo: https://stackblitz.com/edit/solidjs-templates-xbyq1m?file=src/use-presence.ts |
|
Here the getters subscribe to any reactive change of state(), which is significantly less performant than using createSelector(). This is a low hanging fruit for optimization. |
Exactly. We can also encode status as union of numbers or single characters to shave off some bits. I wanted to see the initial reaction. I will open a PR if you or other maintainers say OK. |
|
I like the @snnsnn idea very much. We should aim to do as little work as possible inside these primitives, whether something should be kept in a separate signal to not over-execute is the users' decision. If we can maintain the state using a single signal, either by using an enum like @snnsnn suggested, or a plain object that is updated by restructuring. We have utilities like As for the shape of the returned object, it either should be an object with getters, just like a store/props. Or a single signal (memo) returning the derived object state in an immutable way. with getters declare function createPresence(options): {
readonly isMounted: boolean;
readonly entering: boolean;
readonly entered: boolean;
readonly exiting: boolean;
readonly exited: boolean;
}
const presense = createPresense(/* ... */)
createEffect(() => {
console.log(presense.entering)
})
// manually memoize if needed
const entering = createMemo(() => presense.entering)a single signal declare function createPresence(options): Accessor<{
readonly isMounted: boolean;
readonly entering: boolean;
readonly entered: boolean;
readonly exiting: boolean;
readonly exited: boolean;
}>
const presense = createPresense(/* ... */)
createEffect(() => {
console.log(presense().entering)
})
// manually memoize if needed
const entering = createMemo(() => presense().entering)@atk I think it's up to the user if he needs to optimize or not. It's better to return the enum signal directly from the primitive and let the user decide how to structure it further instead of optimizing prematurely with selectors for each property. But what is more important than optimizations is meeting user expectations. |
|
Creating objects is definitely slower than just using a string out of a union. |
|
I like the idea of exposing the state directly: import { Accessor, createComputed, createSignal, on, untrack } from 'solid-js';
interface Options {
initialTransition?: boolean;
duration: number | { enter: number; exit: number };
}
type State = 'entering' | 'entered' | 'exiting' | 'exited';
export function usePresence(
show: Accessor<boolean>,
options: Options,
): [Accessor<boolean>, Accessor<State>] {
const { initialTransition, duration } = options;
const enter = typeof duration === 'number' ? duration : duration.enter;
const exit = typeof duration === 'number' ? duration : duration.exit;
const initialVal = untrack(show) ? 'entered' : 'exited';
const [state, setState] = createSignal<State>(initialVal);
const handleTransition = () => {
const val = show();
setState(val ? 'entering' : 'exiting');
setTimeout(() => setState(val ? 'entered' : 'exited'), val ? enter : exit);
};
if (initialTransition) {
handleTransition();
}
createComputed(on(show, handleTransition, { defer: true }));
const isMounted = () => state() !== 'exited';
return [isMounted, state];
}Now we can take advantage of array destructuring for easier re-naming: const [isMounted, state] = usePresence(show, {
duration: 1000,
initialTransition: true,
});I like the idea of returning an array but I am not sure if it is best to return an object or an array. What do you think? And about the memoization, we don't need memoization because transitioning from <Show when={isMounted()}>
<div
style={{
transition: 'all .5s linear',
// Transitioning from false to false has no effect
...(state() === 'entering' && {
color: 'green',
}),
...(state() === 'entered' && {
color: 'red',
}),
...(state() === 'exiting' && {
color: 'orange',
}),
...(state() === 'exiting' && {
color: 'blue',
}),
}}
>
Some Content!!
</div>
</Show>Even if we use memo, we will run this computation inside the memo function, so in terms of net computation work there is no difference. I believe it is best to run the computation when it is needed rather than beforehand. |
|
You could even use it in a branchless version <Show when={isMounted()}>
<div
style={{
transition: 'all .5s linear',
color: ({
entering: 'green',
entered: 'red',
exiting: 'orange',
exited: 'blue'
})[state()]
}}
>Some Content!!</div>
</Show>I'd probably prefer a CSS variable, because that helps the browser optimize the color changes, but this should work, too. |
Array destructuring is usually used to separate reads from writes, state from actions, I don't see a point in returning const state = createPresense(/* ... */)
const isMounted = createMemo(() => state() !== 'exited')
const isSelectedState = createSelector(state)
createEffect(() => {
if (isSelectedState('entering')) { /* ... */ }
}) |
Good point. I don't like redundancies either. One last question, about the naming, presense is a facy name but does not convey the true nature of what we are doing here and is plainly confusing if you are not familiar with it. Can we come up with a better name? |
|
I'm only familiar with the naming from the work on motionone, and here it's used to describe a similar pattern, so I quite like it. |
Name is good, memorable and distinct, my concern is its connotations, which is also stressed further by the proposed description, "A small SolidJS utility to animate the presence of an element" gives you the idea of interacting with the underlying DOM element, all we do is delay the unmounting of the element through a derived signal. Maybe it is the function name we need to find an alternative. How about |
|
I'm looking at it as
The usage with DOM elements should be only brought up as an example of one usecase. There are probably other places in the documentation to make the boundaries clear. |
Yeah, this may make more sense since we are aiming to apply animations both when mounting and unmounting, while the name OK, thanks. I will send a PR. |
|
Apologies for not being in on this conversation all day. I've been trying to keep track of it, but I have an upcoming deadlines for official work stuff which generally makes me unavailable for OSS stuff during 9-5EST, but I wanted to make sure I got in on this discussion before a PR was opened against my fork. I want to say that I have no problem with changing the API itself. The value of this package, to me, is to have an exposed state-primitive (or set of state primitives) that can represent the presence of elements/components/data. However, exactly how it is exposed to the end user is totally up for debate in this PR. @snnsnn @thetarnav I do like the idea of turning this effectively into a strict state machine, but I also have a few concerns with how existing paradigms that are useful in While we're already considering changes to the API, I figure this one might be worth bringing to the table as well. I originally contributed the |
Honestly, it was one of my initial impressions when looking at the implementation as well. "Can't I just pass a And about the other proposed changes: the same things that were possible with the previous API, should be possible with this one too. When making it into a pure state-machine, how do we let the user force animations to happen, which is done by forcing a reflow internally now. I think we need to establish what the primitive is supposed to do exactly, test it, and publish it as the initial implementation. Then we can switch the implementation and even the API details all we want, as long as the same things are still possible. |
@EthanStandel Sorry but I could not understand your concern. Is it synronizing enter and exit transitions for multiple elements or executing in-left/out-right movement? State machine offers far greater flexibility than manually swapping states. You can derive any state from existing states like we did for https://stackblitz.com/edit/solidjs-templates-2kjppq?file=src/App.tsx In your implementation you have too many moving pieces and too many manual updates which will degrade performance considerably. Solid is fast but still lags if you have too many components. Also, you don't batch update, which triggers undesired intermediate state updates. |
Here is my understanding, which was written for the README file: A small utility to have stronger control over an element's presense in the DOM in order to apply enter and exit transitions. When an elements visibilty tied to a signal, the elements gets mounted and unmounted abruptly, not permitting to apply any css transitions. <Show when={show()}>
<div></div>
</Show>Through const [show, setShow] = createSignal(true);
const state = usePresence(show, {
duration: 500,
initialTransition: true,
});
createEffect(() => console.log(state()));This allows us to apply enter and exit transitions to an element but first we need to hand over the control of the element's visibilty to the const isMounted = () => state() !== 'exited';
<Show when={isMounted()}>
<div></div>
</Show>Now, when When <div
style={{
transition: 'all .5s linear',
...(state() === 'entering' && {
color: 'green',
}),
...(state() === 'entered' && {
color: 'red',
}),
}}
>
Some Content!!
</div>Setting <div
style={{
transition: 'all .5s linear',
...(state() === 'exiting' && {
color: 'orange',
}),
...(state() === 'exited' && {
color: 'blue',
}),
}}
>
Some Content!!
</div> |
|
@thetarnav I'd be happy to just combine them! When this was a React hook there was more risk of lost frames from triggering unnecessary state updates from the extra effect, but in Solid the user has more control over what a state update does and the state updates themselves are generally far less weighty. So I don't believe this actually is a relevant issue here. @snnsnn Your understanding of the model is definitely correct, but the new StackBlitz you shared isn't doing what you think it is. If you expand the transition time to be something longer, it's a little bit easier to see the problem. When you trigger the presence of an element to enter, it's immediately rendering in a hidden state and then waiting I think that if you just added one more state (I think it's just one) to the flow, it could cover for this. Here's a janky fork of the previous example with a potential fix. I got away with just awaiting |
I see, timing and triggering transitions was different in my initial implementation but that is lost somewhere between the iterations. I will fix it. Thank you. |
|
Ok, I had time to look at it with a rested mind. The problem is, as you said, the element is mounted with the initial state and remain there until In my earlier initial implementation (not shown in the discussion) did not suffer from this, because the element was mounted with setState('entering');
setTimeout(() => setState('entered'), 0);About the solution, it is simple, we add an initial state. Initial state should be there from the beginning because an element can be in one of these tree resting states, Here is a fixed demo: https://stackblitz.com/edit/solidjs-templates-d6rfxr?file=src/use-presence.ts Now having the initial state, element will be mounted with <div
style={{
background: 'gray',
transition: 'all 3s linear',
...(state() === 'initial' && {
"background-color": 'red',
opacity: '1',
transform: 'translateX(-25px)',
}),
...(state() === 'entering' && {
opacity: '1',
"background-color": 'green',
transform: 'translateX(0)',
}),
...(state() === 'exiting' && {
opacity: '1',
"background-color": 'blue',
transform: 'translateX(25px)',
}),
}}
>
Some Content!!
</div>When show is toggled off, state will move to Without the Your implementation also have similar issues. When we use the values from SecondExample in FirstExample, element does not have left-in right-out transition anymore. I am not sure if that is intentional, it is something I discovered while trying to understand your implementation. https://stackblitz.com/edit/solidjs-templates-jpwjcz?file=src%2FApp.tsx |
|
Since "initial" is basically an unmanaged state, it could also be an empty string, which would allow using the state as conditional, e.g. |
|
@snnsnn I like this! I think this fulfills everything that I like about this model and with better performance! I would request that just for the sake of caution, we still have the reflow logic after If we agree that this is the ideal model going forward, do you want to open up a PR to my fork, or should we focus more on merging what I have first and then you can open up a PR with a major version change to update & solidify the API? I just want to make sure that you're credited for your work 😄 @atk I personally prefer that it be legible and clear with exactly what that state is because it is distinctly its own state. I'm not 100% sure about |
Do you mean if we flush our component with a transition state immediately after mounting? React's version mounts elements then sets to updating css properties in another render cycle, but we mount element with initial proprerties, then we flush our component with transition so to have the same effect.
That is OK for me if maintainers OK with that. |
|
@snnsnn I don't mean anything from a state/Solid perspective. I mean it specifically from a DOM perspective to ensure that the I waffled on getting rid of the reflow when I first converted |
Element will be mounted with |
|
@atk @thetarnav @EthanStandel I am somewhat satisfied with the result but still iffy about the passing enter and exit transition durations to You may have notice I had a similar comment deleted but here writing it again. If we use setState('entering');
setTimeout(() => setState('entered'), 0);Then leave everything to css, since css transition will take effect and animate towards the desired state ( If element does not have animation set, there won't be any inconsistencies because element is actually entered to the DOM. The point I am trying to make is, element's css will never be in an inconsistent state. This way, the state reflects the actual status of the element, and we won't loose any functionality compared to the implementation we agreed upon, the one with the On the contrary, our implementation will have one less type State = 'entering' | 'entered' | 'exiting' | 'exited';
export function usePresence(
show: Accessor<boolean>,
options: Options
): Accessor<State> {
const { initialTransition, duration } = options;
const initialVal = untrack(show) ? 'entered' : 'exited';
const [state, setState] = createSignal<State>(initialVal);
let timeout: number;
const handleTransition = () => {
const val = show();
clearTimeout(timeout);
setState(val ? 'entering' : 'exiting');
timeout = setTimeout(() => setState(val ? 'entered' : 'exited'), val ? 0 : duration);
};
// Animate element only when `show` set to true
if (initialTransition && initialVal === 'entered') {
handleTransition();
}
createComputed(on(show, handleTransition, { defer: true }));
return state;
}Here is a working demo: https://stackblitz.com/edit/solidjs-templates-nrye3h?file=src%2Fuse-presence.ts This version closely resembles what https://motion.dev/solid/presence has only they have different naming, However this version will be less flexible in terms of what we can do with the states. With previous implementation we can initialize elements with temporary styles and swap styles during transitions. It can be used for purposes other than css transitions like applying classes to run animations or showing and hiding elements rather than mounting and unmounting them. Simply there is more room for maneuvering. Maybe we can provide both versions and let the user decide. |
|
I like the API of |
It looks very complex. I would prefer something similar to current implementation. Take a look at this: <div
classList={{
'animate-in': state() === 'entering',
'animate-out': state() === 'exiting',
}}
>
Some Content!!
</div>I would keep a simple internal state and and expose a handle to drive the progress or mark it done and let the reactivity do its work rather than setting listeners. |
|
@EthanStandel do you want to add any more changes before the initial release? |
|
Sorry everyone, this week was a product release week for a client. The past two weeks have been really busy, but I'm going to put out some updates tonight to get it in an initial place that I'm happy with. @snnsnn I do get the awkwardness & dislike of having to declare the transition time in both the primitive and in your component. My reccomendation for how people should use this (and how I've used However, if you don't actually wait for the transition time before setting the state to @thetarnav I agree with @snnsnn that I find that model in |
|
@thetarnav I put through updates to address our discussions. I chose not to merge I also had previously only validated whether a switching item should be mounted based on const itemShouldBeMounted = <ItemType>(item: ItemType) => item !== false && item != null;With that, in my opinion this is ready for an initial stage 0 release. Let me know if it needs anything else! |
|
Alright, I'm going to publish the initial version now. Thank you @EthanStandel for bringing this pattern to solid-primitives, I really like the idea and cannot wait to use it more. It could definitely replace the transition-group in some cases while providing a better visual experience. And it's less glitchy too (I think this is not an issue with presence: solidjs-community/solid-transition-group#34). @snnsnn If you have an idea for further improvements you can now make a PR and we can continue the discussion there. I'm really interested in how the API could be simplified further without sacrificing on the functionality. @davedbase Should we share this when you have the time? |
|
@thetarnav I have to travel on a short notice but I will send a PR for basic functionality this weekend. We can improve on it later on. |
This is a proposal to merge my
solid-presence-signalpackage from it's own repo into the @solid-primitives ecosystem. This package is a conversion of a lightweight React animation package called use-presence that I've become partial to (and contributed to) for the high-control it provides developers by simply exposing the animation state as state-primitives.