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

Bad EaseString type #556

Closed
braebo opened this issue Oct 25, 2023 · 4 comments
Closed

Bad EaseString type #556

braebo opened this issue Oct 25, 2023 · 4 comments

Comments

@braebo
Copy link

braebo commented Oct 25, 2023

The EaseString type has string in the union:

| "sine" | "sine.in" | "sine.out" | "sine.inOut" | string;

This widens the type and hides the string literals from intellisense:
Screenshot 2023-10-24 at 9 57 23 PM

This allows any string and the autocomplete is just random globals. It should probably be removed altogether and users should use string literals and the const modifier for typesafety... but a compromise would be changing the type to this:

    //...
    | "sine" | "sine.in" | "sine.out" | "sine.inOut" | ({} & string);

By using ({} & string), the type will still allow arbitrary strings, but now the autocomplete works:

Screenshot 2023-10-24 at 9 56 28 PM

I was going to submit a PR, but it doesn't look like you guys accept those 😅

@jackdoyle
Copy link
Member

Interesting, thanks for the suggestion @fractalhq. I'm not much of a TypeScript guy myself. We definitely need to accept arbitrary strings because that's how CustomEase works (you can assign a name to any ease, or you can even pass 4 comma-delimited bezier numbers).

So to confirm, you're saying that ... | ({} & string) will indeed allow arbitrary strings, so we get the best of both worlds and there won't be backward compatibility issues (like if someone isn't running the latest version of TypeScript)?

@braebo
Copy link
Author

braebo commented Oct 25, 2023

Correct! I'm not positive about backwards compatibility though -- googling a bit lead me to this post https://stackoverflow.com/a/61048124

The last comment claims the fix doesn't work for 3.8, while it does in ^4.x. I'm not sure if that means it simply doesn't fix autocomplete, or if it explicitly introduces a new compiler error. I'd assume the former, but testing it out might be warranted if you want to support v3 (although we're on 5.2 these days).

@jackdoyle
Copy link
Member

Got it, thanks. Seems to still work on 3.x from what I can tell. I'll add this to the next release.

@raulfdm
Copy link

raulfdm commented Nov 11, 2023

For reference: https://github.com/sindresorhus/type-fest/blob/main/source/literal-union.d.ts

jackdoyle added a commit that referenced this issue Nov 30, 2023
- IMPROVED: gsap.context() and gsap.matchMedia() functions will get a 2nd argument that we'll call "contextSafe" which is like a wrapper for any function that you'd like to keep in the context. So any GSAP animations/ScrollTriggers/Draggables/Observers that are created during the execution of that function will be added to the context and the selector text will be scoped.

- IMPROVED: added a suppressEvents parameter to Tween/Timeline/Animation .timeScale() method so that you can optionally prevent it from suppressing events when altering the timeScale. See #551

- IMPROVED added an ignoreSpeed [3rd] parameter to ScrollSmoother's offset() method to allow you to specify whether you want to get the value that corresponds to the window's scroll position or the ScrollSmoother's scrollTop value. See https://greensock.com/forums/topic/35108-problems-with-the-scrollsmoother-plug-in-speed-option-in-gsap-3114/

- IMPROVED: Observer now fires an onStop with an onMove (previously it would only fire after a press). See https://greensock.com/forums/topic/38469-observer-misunderstanding-with-onchange-onmove-and-onstop/

- IMPROVED: slight change to TypeScript definitions for EaseString allows arbitrary strings while also activating code hinting for the common ones in more environments. See #556

- IMPROVED: animations/ScrollTriggers created inside a ScrollTrigger's callback like onEnter/onLeave/onToggle/onEnterBack/onLeaveBack will be added to the original Context (if one existed), meaning selector text will be scoped properly. See https://gsap.com/community/forums/topic/38850-reactgsap-why-i-cant-use-class-selector-in-scrolltrigger-inside-gsapcontext/

- IMPROVED: a ScrollTrigger's snap end position is limited to the resolution of the browser's scroll (whole pixels only), thus if you have a scrubbed animation that's supposed to snap to a very specific spot on that animation, it may end slightly off of that but now a correction runs at the end of the snap to ensure that it gets set PRECISELY to that snapped position. See https://gsap.com/community/forums/topic/38937-scrolltrigger-timeline-snapping-with-label-doesnt-snap-precisely-to-label-position/

- IMPROVED: the gsap-trial files will now work on domains that end in ".local" (for testing only please)

- FIXED: for an Observer, if you pressed, started dragging but released within 3 pixels of the original press, it wouldn't fire the onDragEnd. See https://greensock.com/forums/topic/37510-the-problem-that-ondragend-is-not-called-in-observer/

- FIXED: if you revert() a context/matchMedia that has a reversed animation, it may not render things in the proper order, potentially leaving inline styles when it shouldn't. See https://greensock.com/forums/topic/37432-issues-with-horizontalloop-helper/

- FIXED: if Draggable is applied to an <object> where the document isn't defined initially, an error could be thrown. See #549

- FIXED: if you apply a speed to a ScrollSmoother (other than 1), the offset() method would return a value that corresponds to the window's scroll position rather than the ScrollSmoother's scrollTop (which is affected by speed). See https://greensock.com/forums/topic/35108-problems-with-the-scrollsmoother-plug-in-speed-option-in-gsap-3114/

- FIXED: if you apply an onUpdate to a .fromTo() tween, the scope ("this") would be linked to the zero-duration "from" tween instead of the actual tween instance you'd expect, thus its progress() would be 1 instead of 0 at the very start.

- FIXED: if you define position: absolute on a SplitText and then revert(), the width/height inline styles could remain instead of getting cleared out. See https://greensock.com/forums/topic/38391-responsive-behavior-using-splittext/

- FIXED: if you create a ScrollTrigger with a "snap" and pin inside a gsap.matchMedia(), it could lead to incorrect positioning of the pinned element after a resize that makes it no longer match.

- FIXED: if you set a CSS value to "auto" (like height or width) via a GSAP tween and then you revert() it, the original computed value could be left as an inline style instead of cleared out.

- FIXED: if you call kill() or revert() on a MotionPathHelper in certain specific conditions, it may delete the original <path> itself.

- FIXED: worked around a very rare scenario where document.createElement() returns undefined and would consequently throw an error in CSSPlugin. See #553

- FIXED: if you create a staggered animation inside a gsap.context() or gsap.matchMedia() that affects the same element(s) as a gsap.set() that happened before that staggered animation and then the context/matchMedia gets reverted, the initial value may not get reverted properly.

- FIXED: if you animate a percentage-based width/height of an element whose parent has padding and/or is flexbox/grid, and the target element doesn't have that property already set inline, it may miscalculate the starting value of the tween. Related to https://gsap.com/community/forums/topic/38599-how-to-animate-object-fit/

- FIXED: if you create multiple SplitText instances on the same element inside a gsap.context() and then revert() that Context, it may not fully revert the element to its original state. See https://gsap.com/community/forums/topic/38734-splittext-innerwrap-renders-twice-in-strictmode/

- FIXED: if you set allowNestedScroll: true in the ScrollTrigger.normalizeScroll() feature, touch-scrolling on a link on a mobile device could result in a click event firing on that link. See https://gsap.com/community/forums/topic/38770-why-does-the-burger-menu-scroll-along-with-the-content-on-mobile/#comment-193009

- FIXED: if you set end: "max" or clamp() the end of a ScrollTrigger that has a pin, and the pinSpacing was extending the page taller, the dynamic adjustment of the end value would also affect the pinSpacing, reducing the maximum scroll area which then wouldn't be accurately reflected in the final end value.

- FIXED: if you define a stagger with grid: "auto" on an Array of elements that don't wrap at all (not really a grid), the last element's timing wouldn't be correct. See https://gsap.com/community/forums/topic/38536-when-scrolling-down-a-batch-the-stagger-doesnt-seem-to-work-correctly/

- FIXED: if you clamp() a ScrollTrigger's start value and it would naturally (without clamping) resolve to beyond the maximum scroll position, it wouldn't get clamped. Starting values were only clamped such that they weren't allowed to be negative (focused on the top of the page only, not the bottom too).

- FIXED: if you dynamically added/created a ScrollTrigger while ScrollSmoother was mid-scrub, it could lead to the scroll jumping. See https://gsap.com/community/forums/topic/37515-dynamic-scrolltrigger-with-pin-inside-a-scrollsmoother/

- FIXED: if you run SplitText on text that has words separated by non-breaking spaces (&nbsp;), it wouldn't recognize that as a word delimiter. See https://gsap.com/community/forums/topic/37271-why-does-the-splitted-node-from-splittext-includes-extra-whitespace-note-only-the-node-which-is-next-tag/

- FIXED: Flip.fit() is now gsap.context()-aware so that it'll revert inline styles when the context is reverted.

- FIXED: regression in 3.11 that could cause transformOrigin to lose its "z" portion if you apply it in a .from() or .fromTo() tween or a context that gets reverted. See https://gsap.com/community/forums/topic/38958-rotatey-not-working-in-nuxt/

- FIXED: if you call gsap.registerPlugin(Observer) multiple times, it could throw an error.
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

No branches or pull requests

3 participants