Skip to content

Remove pools key and proxy#1

Open
Fryuni wants to merge 3 commits intowarpify:mainfrom
Fryuni:generalize-pool
Open

Remove pools key and proxy#1
Fryuni wants to merge 3 commits intowarpify:mainfrom
Fryuni:generalize-pool

Conversation

@Fryuni
Copy link

@Fryuni Fryuni commented Apr 19, 2024

Describe your changes

  • Remove the need to provide a unique key to the pool
  • Allow adding timelines to the pool
  • Add functional scope API to allow using a timeline and adding it back to the pool:
    withTimeline(timeline => {
      timeline
        .to({...})
        .play();
    });

@zanhk
Copy link
Member

zanhk commented Apr 20, 2024

Thanks for the PR @Fryuni.

Unfortunately with this implementation withTimeline was not working for me, do you tested in a particular scenario so I can try to reproduce it? (using just useTimeline probably doesn't fix the issue as a new timeline is created every time), before investigating further I think that there are few points to address in the general implementation:

  • Right now plugins doesn't seem to be working, even tho I added gsap through peerDependencies, maybe i need to register the plugins inside the module?
  • I think that adding additional syntax on top could be confusing and maybe we can find a way to use it with minimal changes to the existing code

A member from the gsap team suggested to gsap.context() but I m not seeing how this could help in this case, maybe you have some ideas (https://gsap.com/community/forums/topic/40524-gsap-with-astro-view-transitions-integration/#comment-201800).

Starting from the what I think the ideal solution could be (idk if it's possible):

import gsap from "astro-gsap";

gsap.timeline()
	.set("[data-hero-text-reveal]", { opacity: 1 })
	.from(childSplit.lines, {
		yPercent: 300,
		skewY: 7,
		stagger: 0.2,
	})
	.to(
		"[data-hero-reveal]",
		{
			opacity: 1,
			stagger: 0.1,
		},
		"<=",
	);

so basically using the same syntax as the original gsap, but using a wrapper that will handle the timeline pool by overriding the gsap.timeline function (or other function that may leave same traces).

you may register it like this

import { defineConfig } from 'astro/config';
import gsap from "astro-gsap/integration";

import { ScrollTrigger } from "gsap/ScrollTrigger";
import { SplitText } from "gsap/SplitText";

// https://astro.build/config
export default defineConfig({
	integrations: [
		gsap({
			plugins: [ScrollTrigger, SplitText],
			options: {},
		}),
	],
});

Then under the hood we override the gsap.timeline function to use pooled timelines

(pseudo code, not working)

import gsap from "gsap";

type Timeline = gsap.core.Timeline;
type TimelineOptions = Omit<gsap.TimelineVars, "paused">;

const pool = new Set<Timeline>();

...

// Override the gsap.timeline function to use pooled timelines
// Extend gsap with a custom timeline function that uses pooling
const customGsap = {
	...gsap,
	timeline: (options?: TimelineOptions) => {
		const timeline: gsap.core.Timeline = useTimeline(options);

		let clearAdded = false;

		const handler: ProxyHandler<gsap.core.Timeline> = {
			get(
				target: gsap.core.Timeline,
				prop: PropertyKey,
				receiver: unknown,
			): unknown {
				const origMethod = target[prop as keyof gsap.core.Timeline];
				if (typeof origMethod === "function") {
					return (...args: unknown[]) => {
						if (!clearAdded) {
							target.clear();
							clearAdded = true;
						}
						const result = origMethod.apply(target, args);

						if (prop === "play") {
							clearAdded = false;
						}

						// Ensure the proxy maintains the correct this context
						return result === target ? receiver : result;
					};
				}
				return origMethod;
			},
		};

		return new Proxy(timeline, handler);
	},
};

// Export the customized gsap module
export default customGsap;

At this point the user just have to change one line of code to get it working:

- import gsap from "gsap";
+ import gsap from "astro-gsap";

What do you think and what flaws do you see in this approach?

@zanhk
Copy link
Member

zanhk commented Apr 20, 2024

At this point maybe we could simplify the whole thing by killing the timelines once the page is about to being swapped

import gsap from "gsap";

type Timeline = gsap.core.Timeline;
export const pool = new Set<Timeline>();

function useTimeline(options?: gsap.TimelineVars): Timeline {
	const timeline = gsap.timeline(options);
	// add the timeline to the pool
	pool.add(timeline);
	return timeline;
}

document.addEventListener("astro:before-swap", (ev) => {
	// before swapping with a new page clean all the timelines
	for (const timeline of pool.values()) {
		timeline.kill();
	}
	pool.clear();
});

const customGsap = {
	...gsap,
	timeline: (options?: gsap.TimelineVars) => {
		const timeline: Timeline = useTimeline(options);

		const handler: ProxyHandler<gsap.core.Timeline> = {
			get(
				target: gsap.core.Timeline,
				prop: keyof gsap.core.Timeline,
				receiver: unknown,
			): unknown {
				const origMethod = target[prop];
				if (typeof origMethod === "function") {
					return (...args: unknown[]) => {
						const result = origMethod.apply(target, args);
						return result === target ? receiver : result;
					};
				}
				return origMethod;
			},
		};

		return new Proxy(timeline, handler);
	},
};

// Export the customized gsap module
export default customGsap;

What do you think?

@Fryuni
Copy link
Author

Fryuni commented Apr 20, 2024

In that case it is no longer a pool right?! It is an Astro-aware timeline.

I don't see the purpose of the proxy. Since Timeline is a normal class it could just be extended and the fluid API would work automatically (return this respects subclassing).

Then our timeline function could just return a new instance of our timeline instead of the default.

@zanhk
Copy link
Member

zanhk commented Apr 20, 2024

In that case it is no longer a pool right?! It is an Astro-aware timeline.

I don't see the purpose of the proxy. Since Timeline is a normal class it could just be extended and the fluid API would work automatically (return this respects subclassing).

Then our timeline function could just return a new instance of our timeline instead of the default.

You are right, you mean something like this

import gsap from "gsap";

type Timeline = gsap.core.Timeline;
export const pool = new Set<Timeline>();

document.addEventListener("astro:before-swap", (ev) => {
	// before swapping with a new page clean all the timelines
	for (const timeline of pool.values()) {
		timeline.kill();
	}
	pool.clear();
});

class TimelinesRecycler extends gsap.core.Timeline {
	constructor(vars?: gsap.TimelineVars, time?: number) {
		super(vars, time);
		pool.add(this);
	}
}

const customGsap = {
	...gsap,
	timeline: (vars?: gsap.TimelineVars) => {
		return new TimelinesRecycler(vars);
	},
};

export default customGsap;

correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants