Tell custom transitions if they're in or out

[tivac]

Is your feature request related to a problem? Please describe.
We did some work recently to replace the default transitions with custom ones to avoid the injection/removal of custom @keyframes rules into a stylesheet every time we run a svelte transition. It causes at least one full-page repaint every time, which for game UI is real rough.

Our custom transitions don't use css or tick, they mostly take the params and apply them to elements using inline style attributes and predefined keyframe animations. That part is all working great.

The challenge is that our custom transitions, because they don't use css or tick, don't know which direction they should be transitioning, it's not passed as part of the params.

https://svelte.dev/repl/8297026848b64192ad8faa46120c9700?version=3.14.1

Describe the solution you'd like
I'd like a new property added to the params object passed to transition functions. I don't have a strong opinion about what it should be called. The property would have 3 possible values, in, out, both.

Describe alternatives you've considered
Our current solution is to define a transition function that takes an extra argument, then export bound versions of that function where the first argument is either in or out. This works ok, but is awkward and annoying to have to use multiple imports if you want to fade an element in or out.

Given how our transitions work and how svelte calls transition functions we still wouldn't be able to use transition:custom, but that's fine. No longer need to import { inFade, outFade } from "transitions" would be a big step in the right direction and simplify our lives appreciably.

How important is this feature to you?
This isn't a deal-breaker, we have a workaround. It's just unpleasant and slightly error-prone at the moment since it's possible to assign the wrong transition to either the in: or out:, whereas with this proposal it would work even more like the transitions that are built into svelte.

[mrkishi]

Having faced a similar issue, I believe this would be a good addition.

Instead of modifying the params argument, it'd probably make sense to add a third parameter so the change is backwards compatible and params continues to be the user-provided parameters as-is, as well as also providing the direction for transitions that are created just-in-time by the returned function (I'm calling them transition generators):

type TransitionGenerator = (direction: 'in' | 'out') => TransitionConfig;
type TransitionSetup = (node: Element, params: any, direction: 'in' | 'out' | 'both') =>
    TransitionConfig | TransitionGenerator;

While it expands the transition contract we have to support, I believe it's a small enough change that it's worth it.

cc @Rich-Harris @pngwn, do you have any opinions since you've dealt quite a bit with transitions?

[tivac]

If you're going to add a new 3rd param it should be an object, not just a single boolean. That way future additions aren't breaking changes.

I don't see any issues with adding it to params, but so long as the support gets added I'm good. If someone official-ish is willing to weigh in on their feelings yay/nay on this I'm definitely willing to at least try and do the legwork.

[mrkishi]

Unfortunately, adding it to params would be a breaking change.

[tivac]

... why?

[mrkishi]

Unless I'm confusing things, users can be already using params.name_we_pick for their own transition settings.

[Conduitry]

If we favor the user-specified params over direction: 'in' or direction: 'out', I think we could get away with adding this there and not calling it a breaking change, but the extra control given to consumers there (by letting them override the value) might be dangerous. It does seem simpler to split up 'params from the transition consumer' and 'params from the framework'. Making the extra param an object so that we can potentially add more later does seem like a good idea.

[mrkishi]

I thought about merging too but even then it'd be a breaking change. For instance, I've used direction myself as a transition parameter, but it was optional, so if we did that I could start getting an 'in' where that's not meaningful.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[emmbm]

Any update on this issue? If naming the param direction is too prone to overlapping user-specified params and breaking things, could we instead consider { ..., in: boolean, out: boolean } where, for example, a transition bound such as

<p in:customTransition>Paragraph</p>

would pass { ..., in: true, out: false } in its params, and

<p transition:customTransition>Paragraph</p>

would pass { ..., in: true, out: true } in its params.

I feel like this is a justifiable approach and developpers who would see a breaking change there should be more concerned with the naming of their transition function's params.

[tivac]

@iolyd There's a PR from @mrkishi, #4019

but it's stale and is missing docs & tests. I might take a look at catching it back up and filling in the missing details though because I suspect I'm still gonna need this behavior again before too long here.

[Conduitry]

This should be (finally!) available in 3.54.0.

[robertadamsonsmith]

I had hoped that I would be able to determine the direction of the current transition (i.e. transition:mytransition), by using the new options.direction value, but I see that it is always set to 'both' for a transition. On other words, options.direction tells me how the transition has been bound (in/out/both), rather than the direction currently being played (in/out). It seems to me that knowing the direction currently being played is what is generally needed so that instead of writing this (which works, but is repetitious):

<div in:mytransition={transition_options} out:mytransition={transition_options} />

I can write this:

<div transition:mytransition={transition_options} />

And then in mytransition, I want the options.direction value to always be either 'in' or 'out' (never 'both'), depending on the direction of the currently played transition.

This would make it possible for things like crossfade animations, to be used much more simply.

(It could be that I misunderstand how this can be determined already, or why this is a bad idea)

[tivac]

I pondered that for a hot minute but didn't do it because the previous PR for this in #4019 just passed the initial parameter and I was trying to minimize the change.

In retrospect that was a mistake 😞

I agree that custom transition functions returned from the config function, when actually run, should get passed the current direction of the transition. The config function should still get in/out/both, but if it returns a function that should only get one of in/out when called.

Can we consider this a bug? I'd be happy to put up a PR to fix it if we can 🤔

It looks like it'd be a very small change, we'd need to grab the b value that is passed in, then use it as a conditional to determine what string to pass as part of a new options object instead of reusing the existing options when calling config(options).

[robertadamsonsmith]

I can't think of a situation where it would be useful for direction to be the config value instead of the current value, and so I think that a minor breaking change would be appropriate unless there are examples to the contrary.

[tivac]

I can't think of a situation where it would be useful for direction to be the config value instead of the current value, and so I think that a minor breaking change would be appropriate unless there are examples to the contrary.

Would you be willing to open a new issue with your take so we can hopefully get some maintainer eyes on this and verify if it's something they'd accept?

[tivac]

@Conduitry can we get your ruling on whether or not this could be considered a bug that can be fixed via a patch release? If you think it'd be a breaking change I get it, but it seems so obvious now and I'm annoyed I didn't catch it earlier.