From c9c6ea78547b8614e466d6e2430406de08525b5e Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Sun, 31 Jan 2021 16:52:57 +0100 Subject: [PATCH] (fix) fallback param for built-in transition/animate (#790) 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 --- .../htmlxtojsx/nodes/animation-directive.ts | 21 +++++++++++++++++-- .../htmlxtojsx/nodes/transition-directive.ts | 19 ++++++++++++++++- .../transition-animate-fallbacks/expected.jsx | 17 +++++++++++++++ .../transition-animate-fallbacks/input.svelte | 17 +++++++++++++++ .../samples/transition-modifiers/expected.jsx | 2 +- 5 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 packages/svelte2tsx/test/htmlx2jsx/samples/transition-animate-fallbacks/expected.jsx create mode 100644 packages/svelte2tsx/test/htmlx2jsx/samples/transition-animate-fallbacks/input.svelte diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/animation-directive.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/animation-directive.ts index 5703435b5..39039448b 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/animation-directive.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/animation-directive.ts @@ -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( @@ -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( @@ -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']); diff --git a/packages/svelte2tsx/src/htmlxtojsx/nodes/transition-directive.ts b/packages/svelte2tsx/src/htmlxtojsx/nodes/transition-directive.ts index 8c4bb59e3..c6ceec688 100644 --- a/packages/svelte2tsx/src/htmlxtojsx/nodes/transition-directive.ts +++ b/packages/svelte2tsx/src/htmlxtojsx/nodes/transition-directive.ts @@ -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; } @@ -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']); diff --git a/packages/svelte2tsx/test/htmlx2jsx/samples/transition-animate-fallbacks/expected.jsx b/packages/svelte2tsx/test/htmlx2jsx/samples/transition-animate-fallbacks/expected.jsx new file mode 100644 index 000000000..bd7d81069 --- /dev/null +++ b/packages/svelte2tsx/test/htmlx2jsx/samples/transition-animate-fallbacks/expected.jsx @@ -0,0 +1,17 @@ +<> +