Skip to content

Error in bidi transitions when returning a function #4974

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

Open
pushkine opened this issue Jun 5, 2020 · 4 comments
Open

Error in bidi transitions when returning a function #4974

pushkine opened this issue Jun 5, 2020 · 4 comments
Labels
awaiting submitter needs a reproduction, or clarification stale-bot temp-stale

Comments

@pushkine
Copy link
Contributor

pushkine commented Jun 5, 2020

https://svelte.dev/repl/283d750225684048b26cf6880dfcea2d?version=3.23.0
Cannot read property 'r' of undefined

if (is_function(config)) {
wait().then(() => {
// @ts-ignore
config = config();
go(b);
});

wait.then resolves after check_outros meaning that outros will be the reassigned to its parent group, which is undefined in this case

@antony
Copy link
Member

antony commented Jun 18, 2020

I'm not sure if this is an issue or not. It certainly causes an error, but I'm not certain that wrapping transitions is supported behaviour.

It certainly might be nice to fix, but I'm holding off on the bug demarcation, for the time being.

@stale
Copy link

stale bot commented Dec 24, 2021

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.

@stale stale bot added the stale-bot label Dec 24, 2021
@tivac
Copy link
Contributor

tivac commented Jan 3, 2024

This is still happening in 4.2.8 and I'm unclear how it could be considered anything other than a bug because while the repro shows a wrapped transition it affects anything that returns a function, a feature specifically supported for transitions.

https://svelte.dev/docs/element-directives#custom-transition-functions

If a transition returns a function instead of a transition object, the function will be called in the next microtask. This allows multiple transitions to coordinate, making crossfade effects possible.

Here's another bug with a much simpler and more realistic repro showing the same issue,

#8224

@tivac
Copy link
Contributor

tivac commented Jan 4, 2024

Wrote up a patch for this that we apply via patch-package, it seems to work fine in my quick local testing. Svelte 4 tests throw up hundreds of failures for me on Windows so dunno that I'll be able to put up a PR for this.

diff --git a/node_modules/svelte/src/runtime/internal/transitions.js b/node_modules/svelte/src/runtime/internal/transitions.js
index 4575e18..f52671b 100644
--- a/node_modules/svelte/src/runtime/internal/transitions.js
+++ b/node_modules/svelte/src/runtime/internal/transitions.js
@@ -321,7 +321,10 @@ export function create_bidirectional_transition(node, fn, params, intro) {
 	 * @param {INTRO | OUTRO} b
 	 * @returns {void}
 	 */
-	function go(b) {
+	function go(b, group) {
 		const {
 			delay = 0,
 			duration = 300,
@@ -339,8 +342,12 @@ export function create_bidirectional_transition(node, fn, params, intro) {
 
 		if (!b) {
 			// @ts-ignore todo: improve typings
-			program.group = outros;
-			outros.r += 1;
+			program.group = group;
+			group.r += 1;
 		}
 
 		if ('inert' in node) {
@@ -413,14 +420,23 @@ export function create_bidirectional_transition(node, fn, params, intro) {
 	return {
 		run(b) {
 			if (is_function(config)) {
+				const group = outros;
+
 				wait().then(() => {
 					const opts = { direction: b ? 'in' : 'out' };
 					// @ts-ignore
 					config = config(opts);
-					go(b);
+					go(b, group);
 				});
 			} else {
-				go(b);
+				go(b, outros);
 			}
 		},
 		end() {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting submitter needs a reproduction, or clarification stale-bot temp-stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants