fix: don't warn with 'Invalid repetitionCount value: 1. Using default: Infinity' for valid modelViewer.appendAnimation(animName, { repetitions: 1 }) (#5178)

It actually just emitted the warning and kept repetitionCount to 1 anyway, the else branch was added in 305e07c

Author: vincentfretin

packages/model-viewer/src/test/three-components/ModelScene-spec.ts

@@ -16,7 +16,7 @@
 import '../renderer-gate.js';
 
 import {expect} from 'chai';
-import {Matrix4, Mesh, SphereGeometry, Vector3} from 'three';
+import {LoopRepeat, Matrix4, Mesh, SphereGeometry, Vector3} from 'three';
 
 import {$scene} from '../../model-viewer-base.js';
 import {ModelViewerElement} from '../../model-viewer.js';
@@ -81,6 +81,51 @@ suite('ModelScene', () => {
       scene.detachAnimation(animationName, false);
       expect(scene.appendedAnimations).to.not.include(animationName);
     });
+
+    suite('appendAnimation repetitionCount validation', () => {
+      // Returns the repetitionCount-related warnings emitted while appending the
+      // first animation with the given repetitionCount.
+      const repetitionWarnings = (repetitionCount: any): string[] => {
+        const warnings: string[] = [];
+        const originalWarn = console.warn;
+        console.warn = (...args: any[]) => void warnings.push(args.join(' '));
+        try {
+          scene.appendAnimation(
+              scene.animationNames[0], LoopRepeat, repetitionCount);
+        } finally {
+          console.warn = originalWarn;
+        }
+        return warnings.filter((w) => w.includes('repetitionCount'));
+      };
+
+      test('a valid numeric value does not warn', () => {
+        expect(repetitionWarnings(1)).to.be.empty;
+      });
+
+      test('Infinity (the default) does not warn', () => {
+        expect(repetitionWarnings(Infinity)).to.be.empty;
+      });
+
+      test('a numeric value < 1 warns', () => {
+        expect(repetitionWarnings(0)).to.have.lengthOf(1);
+      });
+
+      test('NaN warns', () => {
+        expect(repetitionWarnings(NaN)).to.have.lengthOf(1);
+      });
+
+      test('a numeric string >= 1 does not warn', () => {
+        expect(repetitionWarnings('3')).to.be.empty;
+      });
+
+      test('a numeric string < 1 warns', () => {
+        expect(repetitionWarnings('0')).to.have.lengthOf(1);
+      });
+
+      test('a non-numeric string warns', () => {
+        expect(repetitionWarnings('abc')).to.have.lengthOf(1);
+      });
+    });
   });
 
   suite('setSize', () => {

packages/model-viewer/src/three-components/ModelScene.ts

@@ -990,13 +990,17 @@ export class ModelScene extends Scene {
         }
         repetitionCount = Math.max(parseInt(repetitionCount), 1);
       }
-    } else if (typeof repetitionCount === 'number' && repetitionCount < 1) {
-      repetitionCount = 1;
-      console.warn(`Invalid repetitionCount value: ${
-          repetitionCount}. Using  1 value as minimum.`);
+    } else if (typeof repetitionCount === 'number' && !isNaN(repetitionCount)) {
+      // A valid number >= 1 (including Infinity) is left untouched.
+      if (repetitionCount < 1) {
+        console.warn(`Invalid repetitionCount value: ${
+            repetitionCount}. Using 1 as minimum.`);
+        repetitionCount = 1;
+      }
     } else {
       console.warn(`Invalid repetitionCount value: ${
           repetitionCount}. Using default: Infinity`);
+      repetitionCount = Infinity;
     }
 
     if (repetitionCount === 1 && loopMode !== LoopOnce) {