Skip to content

Commit

Permalink
(fix) fallback param for built-in transition/animate (#790)
Browse files Browse the repository at this point in the history
Up to Svelte version 3.32.0, the built-in animate/transition functions have optional parameters, but according to its typings they were mandatory. To not show unnecessary type errors to those users, `{}` should be added as a fallback parameter if the user did not provide one.
It may be the case that someone has a custom animation with the same name that expects different parameters, but that possibility is far less likely.
#785
  • Loading branch information
dummdidumm authored Jan 31, 2021
1 parent 611e665 commit c9c6ea7
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 4 deletions.
21 changes: 19 additions & 2 deletions packages/svelte2tsx/src/htmlxtojsx/nodes/animation-directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Node } from 'estree-walker';
import { isQuote } from '../utils/node-utils';

/**
* animation:xxx(yyy) ---> {...__sveltets_ensureAnimation(xxx(__sveltets_ElementNode,__sveltets_AnimationMove,(yyy)))}
* animate:xxx(yyy) ---> {...__sveltets_ensureAnimation(xxx(__sveltets_ElementNode,__sveltets_AnimationMove,(yyy)))}
*/
export function handleAnimateDirective(htmlx: string, str: MagicString, attr: Node): void {
str.overwrite(
Expand All @@ -13,7 +13,11 @@ export function handleAnimateDirective(htmlx: string, str: MagicString, attr: No
);

if (!attr.expression) {
str.appendLeft(attr.end, '(__sveltets_ElementNode,__sveltets_AnimationMove))}');
if (animationsThatNeedParam.has(attr.name)) {
str.appendLeft(attr.end, '(__sveltets_ElementNode,__sveltets_AnimationMove,{}))}');
} else {
str.appendLeft(attr.end, '(__sveltets_ElementNode,__sveltets_AnimationMove))}');
}
return;
}
str.overwrite(
Expand All @@ -26,3 +30,16 @@ export function handleAnimateDirective(htmlx: string, str: MagicString, attr: No
str.remove(attr.end - 1, attr.end);
}
}

/**
* Up to Svelte version 3.32.0, the following built-in animate functions have
* optional parameters, but according to its typings they were mandatory.
* To not show unnecessary type errors to those users, `{}` should be added
* as a fallback parameter if the user did not provide one.
* It may be the case that someone has a custom animation with the same name
* that expects different parameters, or that someone did an import alias fly as foo,
* but those are very unlikely.
*
* Remove this "hack" some day.
*/
const animationsThatNeedParam = new Set(['flip']);
19 changes: 18 additions & 1 deletion packages/svelte2tsx/src/htmlxtojsx/nodes/transition-directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ export function handleTransitionDirective(htmlx: string, str: MagicString, attr:
}

if (!attr.expression) {
str.appendLeft(attr.end, '(__sveltets_ElementNode))}');
if (transitionsThatNeedParam.has(attr.name)) {
str.appendLeft(attr.end, '(__sveltets_ElementNode,{}))}');
} else {
str.appendLeft(attr.end, '(__sveltets_ElementNode))}');
}
return;
}

Expand All @@ -32,3 +36,16 @@ export function handleTransitionDirective(htmlx: string, str: MagicString, attr:
str.remove(attr.end - 1, attr.end);
}
}

/**
* Up to Svelte version 3.32.0, the following built-in transition functions have
* optional parameters, but according to its typings they were mandatory.
* To not show unnecessary type errors to those users, `{}` should be added
* as a fallback parameter if the user did not provide one.
* It may be the case that someone has a custom transition with the same name
* that expects different parameters, or that someone did an import alias fly as foo,
* but those are very unlikely.
*
* Remove this "hack" some day.
*/
const transitionsThatNeedParam = new Set(['blur', 'fade', 'fly', 'slide', 'scale', 'draw']);
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<>
<h1 {...__sveltets_ensureTransition(blur(__sveltets_ElementNode,{}))}>Hello</h1>
<h1 {...__sveltets_ensureTransition(fade(__sveltets_ElementNode,{}))}>Hello</h1>
<h1 {...__sveltets_ensureTransition(fly(__sveltets_ElementNode,{}))}>Hello</h1>
<h1 {...__sveltets_ensureTransition(slide(__sveltets_ElementNode,{}))}>Hello</h1>
<h1 {...__sveltets_ensureTransition(scale(__sveltets_ElementNode,{}))}>Hello</h1>
<h1 {...__sveltets_ensureTransition(draw(__sveltets_ElementNode,{}))}>Hello</h1>

<h1 {...__sveltets_ensureTransition(blur(__sveltets_ElementNode,{}))}>Hello</h1>
<h1 {...__sveltets_ensureTransition(blur(__sveltets_ElementNode,{}))}>Hello</h1>

<h1 {...__sveltets_ensureAnimation(flip(__sveltets_ElementNode,__sveltets_AnimationMove,{}))}>Hello</h1>

<h1 {...__sveltets_ensureTransition(foo(__sveltets_ElementNode))}>Hello</h1>
<h1 {...__sveltets_ensureTransition(foo(__sveltets_ElementNode))}>Hello</h1>
<h1 {...__sveltets_ensureTransition(foo(__sveltets_ElementNode))}>Hello</h1>
<h1 {...__sveltets_ensureAnimation(foo(__sveltets_ElementNode,__sveltets_AnimationMove))}>Hello</h1></>
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<!-- transitions -->
<h1 transition:blur>Hello</h1>
<h1 transition:fade>Hello</h1>
<h1 transition:fly>Hello</h1>
<h1 transition:slide>Hello</h1>
<h1 transition:scale>Hello</h1>
<h1 transition:draw>Hello</h1>
<!-- also for in/out transitions -->
<h1 in:blur>Hello</h1>
<h1 out:blur>Hello</h1>
<!-- animate -->
<h1 animate:flip>Hello</h1>
<!-- not others -->
<h1 transition:foo>Hello</h1>
<h1 in:foo>Hello</h1>
<h1 out:foo>Hello</h1>
<h1 animate:foo>Hello</h1>
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<><div {...__sveltets_ensureTransition(slide(__sveltets_ElementNode))}>
<><div {...__sveltets_ensureTransition(slide(__sveltets_ElementNode,{}))}>
{item}
</div></>

0 comments on commit c9c6ea7

Please sign in to comment.