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

Transitions fix #4742

Closed
wants to merge 38 commits into from
Closed

Transitions fix #4742

wants to merge 38 commits into from

Conversation

pushkine
Copy link
Contributor

@pushkine pushkine commented Apr 28, 2020

Implements https://twitter.com/pushkine_/status/1254903272374231042
Resolves #4344
Fixes #4732
Fixes #4842
Fixes #4794
Fixes #4787
Fixes #4784
Fixes #4831

Changelog

Package

  • Bumped all dependencies except for css-tree and periscopic as they break the compiler.

Rollup config

  • DRY, comments
  • Declare __VERSION__ as a global & replace that instead of the quoted version of it

Tests

  • Ported tests to typescript
  • added npm script to update expected
  • added dependencies to run tests : ts-mocha, esm
  • added dependencies : @types/jsdom, @types/puppeteer
  • inlined dependency tiny-glob

Compiler

  • Inlined a dozen runtime helpers
  • Removed legacy compile option as it was only used once in the whole thing

Each

  • Removed array literal optimization
    Providing an alternate compiler path specifically for array literals ( i.e. arrays with the fewest amount of items ) is just not worth the amount of additional code and potential bugs
  • Added helpers for for loops & if elses

Animations / Transitions

  • DRY, fixed various glitches
  • strategy : new parameter "reverse" (default) | "mirror" .
    Defines the way transition easing is computed
    reverse always eases from 0 to 1 no matter the direction,
    mirror goes from 0 to 1 on intro and 1 to 0 on outro

Runtime

svelte/internal

  • DRY, optimizations

svelte/easing

  • DRY, optimizations
  • Added easeIn, easeOut, easeInOut as aliases of quartIn, quartOut, quartInOut
  • Added cubicBezier

svelte/store

  • Reworked stores into classes to optimize runtime in svelte/motion. Everything is internal, no changes.

BREAKING CHANGES

svelte/motion

Rewrote everything

Springs

  • The spring implementation now uses the damped harmonic oscillation equation, this is how springs are computed on iOS, React Native, Framer Motion, probably Android too.
  • Breaking : damping now ranges from 0.1 to 20, stiffness now ranges from 10 to 200, soft and hard are not a thing anymore
  • mass : new parameter, scales the equation uniformly, see graphing for reference.
  • soft : new parameter, forces the spring to match a critically damped oscillation, i.e. gently transitions without oscillating.
  • .setImmediate : new method, sets value immediately, sets velocity to 0.
  • .onRest : new method, the passed callback is called every time the spring settles, returns an unsubscriber.
  • .set : does not return a Promise anymore, returns a thenable instead. Effectively what this means is that you cannot use .catch on it, everything else works like a Promise would.

Tweens

  • breaking : tweened does not interpolate between dates automatically anymore.
  • .setImmediate : new method, sets value immediately.
  • .onRest : new method, the passed callback is called every time the tween settles, returns an unsubscriber.
  • .set : does not return a Promise anymore, returns a thenable instead. Effectively what this means is that you cannot use .catch on it, everything else works like a Promise would.

New packages

[rolled back] svelte/environment

Environment checks & variables are slowly stacking up and as a front-end framework it only makes sense to expose them

  • globals
  • is_browser : simple window check
  • framerate : milliseconds per frame, used in css keyframe generation as well as in svelte/motion
  • now : performance.now or Date.now depending on the environment
  • is_iframe : true when run inside an iframe.
  • is_cors : true when run inside a cross domain iframe, used in add_resize_listener internally.
  • has_Symbol : true when the environment supports Symbols. Used in a dev check internally.
  • for testing purposes : set_framerate, set_raf, set_now

svelte/interpolate

New package for tween interpolation.

  • numbers : interpolate between numbers.
  • strings : interpolate between strings of constant shape with variable numbers.
  • dates : interpolate between dates.

svelte.dev/examples

Animations

  • Added shuffling while holding spacebar

Springs

Easing Visualizer

What's left

Every test is passing except for a few ones whose folder name has .skip in it. All are seemingly unrelated to the changes, I failed every attempt to fix them, but they are legitimate. Need to write a bunch of tests too, waiting for feedback

@Rich-Harris
Copy link
Member

Thank you so much for this PR, but please avoid adding unrelated changes, and in particular please never commit style changes even if you think they're uncontroversial. I really don't want to reject this PR, it represents a lot of important work, but there's simply no world in which it can be merged in its current state.

A number of the changes are things that already exist in some form (e.g. .setImmediate is expressed by passing { hard: true } as the second argument to spring.set; .onRest is unnecessary as spring.set(...) returns a promise) or would be undesirable (blessing quartOut etc as easeOut isn't the sort of thing we'd do). Some are breaking, and we're not planning to release a v4 any time soon as breaking changes are stressful for users. Others (new packages like svelte/environment) need to go through an RFC process before they become a PR.

Is it possible to break this up into PRs that focus on individual issues? I tried to look at it locally just now to see if I could start isolating parts of it, but even that's difficult because the style changes and TypeScript additions have caused a number of merge conflicts.

@pushkine
Copy link
Contributor Author

pushkine commented May 20, 2020

The PR is still a draft so it's really not meant to be merged as is, I implemented changes optimistically and put it up for feedback, there are still things to do, docs to write and tests to pass

I resolved conflicts and rolled back svelte/environment, it seemed like common sense but I'm not about rfcs and it's not important to me

.onRest is unnecessary as spring.set(...) returns a promise

Ah yes I forgot to write about that, await spring.set() now resolves with boolean true as soon as the spring settles or false as soon as the spring changes its target value

onRest is necessary for spring transitions as the current API does not allow listening to the spring lifecycle without also setting a value to it

blessing quartOut etc as easeOut isn't the sort of thing we'd do

Css and about every easing library out there does have a default easing function, I don't think it's a big deal adding an alias for it.
I'd be interested to learn more about the sort of thing we'd do stuff though, so that I can do more of that in the future

Some are breaking, and we're not planning to release a v4 any time soon as breaking changes are stressful for users.

I just don't think breaking changes for svelte/motion really is that big of a deal. If it was up to me I'd add the breaking change and provide a standalone package of the old implementation

Is it possible to break this up into PRs that focus on individual issues?

Could you list them ? I'm not sure there is as many unrelated changes as it looks like
Transitions are tied to raf loops which are tied to motion, and motion extends stores

A performant solution required a rewrite of each of those, and this PR would make Svelte the most performant runtime animation library by far, at least that I know of. Next on my list was to implement framer's "motion magic", gestures and dragging into svelte, but that's just not doable without the new spring API

@arxpoetica
Copy link
Member

arxpoetica commented May 21, 2020

@pushkine I'm afraid we can't comment on a PR with 300+ file changes. There's just nowhere to start. Please either undo all the style changes, as Rich suggested, or do something to break it up into manageable pieces—multiple PRs or whatever.

I think we're all in agreement that some of these changes overall would probably be fantastic, but it's a complete non-starter with the unwieldy size.

Please undo all unnecessary prettier, eslint, and other style changes as a starting point so we can move any other discussion along.

@pushkine
Copy link
Contributor Author

pushkine commented May 21, 2020

@arxpoetica
the prettier issue is only relevant to ~30 files in src/compiler, I mean sure I can remove prettier to make it easier to review the PR this one time, but going forward I don't know how I feel about contributing if that means I have to give up on style consistency/formatting (#4670)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants