chore(TS) Add more types to Text, BREAKING: change return type of _ge…

…tStyleDeclaration (#9018)
fabricjs · Jun 19, 2023 · cc7b245 · cc7b245
1 parent 5abd37b
commit cc7b245
Show file tree

Hide file tree

Showing 16 changed files with 158 additions and 138 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## [next]
 
+- chore(TS): Remove @ts-nocheck from Text class. [#9018](https://github.com/fabricjs/fabric.js/pull/9018)
 - Fix(Textbox) minimum word width calculation across all lines [#9004](https://github.com/fabricjs/fabric.js/pull/9004)
 - ci(): add Jest for the unit tests [#8919](https://github.com/fabricjs/fabric.js/pull/8919)
 - ci(): Revert "invoke tests after changelog action (#8974)" [#9013](https://github.com/fabricjs/fabric.js/pull/9013)

diff --git a/src/canvas/DOMManagers/CanvasDOMManager.ts b/src/canvas/DOMManagers/CanvasDOMManager.ts
@@ -82,8 +82,8 @@ export class CanvasDOMManager extends StaticCanvasDOMManager {
   ) {
     setStyle(element, {
       position: 'absolute',
-      left: 0,
-      top: 0,
+      left: '0',
+      top: '0',
     });
     allowTouchScrolling(element, allow);
     makeElementUnselectable(element);

diff --git a/src/parser/applyViewboxTransform.ts b/src/parser/applyViewboxTransform.ts
@@ -1,4 +1,4 @@
-//@ts-nocheck
+// @ts-nocheck
 import { svgNS } from './constants';
 import {
   parsePreserveAspectRatioAttribute,
@@ -10,7 +10,7 @@ import { NONE } from '../constants';
  * Add a <g> element that envelop all child elements and makes the viewbox transformMatrix descend on all elements
  */
 
-export function applyViewboxTransform(element) {
+export function applyViewboxTransform(element: HTMLElement) {
   if (!svgViewBoxElementsRegEx.test(element.nodeName)) {
     return {};
   }
@@ -25,13 +25,12 @@ export function applyViewboxTransform(element) {
   const heightAttr = element.getAttribute('height');
   const x = element.getAttribute('x') || 0;
   const y = element.getAttribute('y') || 0;
-  let preserveAspectRatio = element.getAttribute('preserveAspectRatio') || '';
   const missingViewBox =
     !viewBoxAttr || !(viewBoxAttr = viewBoxAttr.match(reViewBoxAttrValue));
   const missingDimAttr =
     !widthAttr || !heightAttr || widthAttr === '100%' || heightAttr === '100%';
   const toBeParsed = missingViewBox && missingDimAttr;
-  const parsedDim = {};
+  const parsedDim: any = {};
   let translateMatrix = '';
   let widthDiff = 0;
   let heightDiff = 0;
@@ -84,7 +83,9 @@ export function applyViewboxTransform(element) {
   }
 
   // default is to preserve aspect ratio
-  preserveAspectRatio = parsePreserveAspectRatioAttribute(preserveAspectRatio);
+  const preserveAspectRatio = parsePreserveAspectRatioAttribute(
+    element.getAttribute('preserveAspectRatio') || ''
+  );
   if (preserveAspectRatio.alignX !== NONE) {
     //translate all container for the effect of Mid, Min, Max
     if (preserveAspectRatio.meetOrSlice === 'meet') {

diff --git a/src/parser/doesSomeParentMatch.ts b/src/parser/doesSomeParentMatch.ts
@@ -1,19 +1,18 @@
-//@ts-nocheck
 import { selectorMatches } from './selectorMatches';
 
-export function doesSomeParentMatch(element, selectors) {
-  let selector,
+export function doesSomeParentMatch(element: HTMLElement, selectors: string[]) {
+  let selector: string,
     parentMatching = true;
   while (
-    element.parentNode &&
-    element.parentNode.nodeType === 1 &&
+    element.parentElement &&
+    element.parentElement.nodeType === 1 &&
     selectors.length
   ) {
     if (parentMatching) {
-      selector = selectors.pop();
+      selector = selectors.pop()!;
     }
-    element = element.parentNode;
-    parentMatching = selectorMatches(element, selector);
+    element = element.parentElement;
+    parentMatching = selectorMatches(element, selector!);
   }
   return selectors.length === 0;
 }
diff --git a/src/parser/getCSSRules.ts b/src/parser/getCSSRules.ts
@@ -1,23 +1,23 @@
-//@ts-nocheck
-
 /**
  * Returns CSS rules for a given SVG document
  * @param {SVGDocument} doc SVG document to parse
  * @return {Object} CSS rules of this document
  */
-export function getCSSRules(doc) {
+export function getCSSRules(doc: Document) {
   const styles = doc.getElementsByTagName('style');
   let i;
   let len;
-  const allRules = {};
+  const allRules: Record<string, Record<string, string>> = {};
   let rules;
 
   // very crude parsing of style contents
   for (i = 0, len = styles.length; i < len; i++) {
-    let styleContents = styles[i].textContent;
+    const styleContents = (styles[i].textContent || '').replace(
+      // remove comments
+      /\/\*[\s\S]*?\*\//g,
+      ''
+    );
 
-    // remove comments
-    styleContents = styleContents.replace(/\/\*[\s\S]*?\*\//g, '');
     if (styleContents.trim() === '') {
       continue;
     }
@@ -32,7 +32,7 @@ export function getCSSRules(doc) {
     // eslint-disable-next-line no-loop-func
     rules.forEach(function (rule) {
       const match = rule.split('{'),
-        ruleObj = {},
+        ruleObj: Record<string, string> = {},
         declaration = match[1].trim(),
         propertyValuePairs = declaration.split(';').filter(function (pair) {
           return pair.trim();
@@ -45,16 +45,15 @@ export function getCSSRules(doc) {
         ruleObj[property] = value;
       }
       rule = match[0].trim();
-      rule.split(',').forEach(function (_rule) {
+      rule.split(',').forEach((_rule) => {
         _rule = _rule.replace(/^svg/i, '').trim();
         if (_rule === '') {
           return;
         }
-        if (allRules[_rule]) {
-          Object.assign(allRules[_rule], ruleObj);
-        } else {
-          allRules[_rule] = Object.assign({}, ruleObj);
-        }
+        allRules[_rule] = {
+          ...(allRules[_rule] || {}),
+          ...ruleObj,
+        };
       });
     });
   }

diff --git a/src/parser/getSvgRegex.ts b/src/parser/getSvgRegex.ts
@@ -1,5 +1,3 @@
-//@ts-nocheck
-
-export function getSvgRegex(arr) {
+export function getSvgRegex(arr: string[]) {
   return new RegExp('^(' + arr.join('|') + ')\\b', 'i');
 }
diff --git a/src/shapes/IText/IText.ts b/src/shapes/IText/IText.ts
@@ -514,7 +514,7 @@ export class IText<
     }
     ctx.fillStyle =
       this.cursorColor ||
-      this.getValueOfPropertyAt(lineIndex, charIndex, 'fill');
+      (this.getValueOfPropertyAt(lineIndex, charIndex, 'fill') as string);
     ctx.globalAlpha = this._currentCursorOpacity;
     ctx.fillRect(
       boundaries.left + boundaries.leftOffset - cursorWidth / 2,
@@ -668,7 +668,7 @@ export class IText<
    * Unused by the library, is for the end user
    * @return {String | TFiller} Character color (fill)
    */
-  getCurrentCharColor(): string | TFiller {
+  getCurrentCharColor(): string | TFiller | null {
     const cp = this._getCurrentCharIndex();
     return this.getValueOfPropertyAt(cp.l, cp.c, 'fill');
   }

diff --git a/src/shapes/Text/StyledText.ts b/src/shapes/Text/StyledText.ts
@@ -10,7 +10,9 @@ import type { StylePropertiesType } from './constants';
 import type { Text } from './Text';
 import { pick } from '../../util';
 
-export type TextStyleDeclaration = Partial<Pick<Text, StylePropertiesType>>;
+export type CompleteTextStyleDeclaration = Pick<Text, StylePropertiesType>;
+
+export type TextStyleDeclaration = Partial<CompleteTextStyleDeclaration>;
 
 export type TextStyle = {
   [line: number | string]: { [char: number | string]: TextStyleDeclaration };
@@ -179,21 +181,18 @@ export abstract class StyledText<
     }
   }
 
-  private _extendStyles(index: number, styles: TextStyleDeclaration) {
+  private _extendStyles(index: number, styles: TextStyleDeclaration): void {
     const { lineIndex, charIndex } = this.get2DCursorLocation(index);
 
     if (!this._getLineStyle(lineIndex)) {
       this._setLineStyle(lineIndex);
     }
 
-    if (!this._getStyleDeclaration(lineIndex, charIndex)) {
+    if (!Object.keys(this._getStyleDeclaration(lineIndex, charIndex)).length) {
       this._setStyleDeclaration(lineIndex, charIndex, {});
     }
 
-    return Object.assign(
-      this._getStyleDeclaration(lineIndex, charIndex) || {},
-      styles
-    );
+    Object.assign(this._getStyleDeclaration(lineIndex, charIndex), styles);
   }
 
   /**
@@ -224,11 +223,9 @@ export abstract class StyledText<
    */
   getStyleAtPosition(position: number, complete?: boolean) {
     const { lineIndex, charIndex } = this.get2DCursorLocation(position);
-    return (
-      (complete
-        ? this.getCompleteStyleDeclaration(lineIndex, charIndex)
-        : this._getStyleDeclaration(lineIndex, charIndex)) || {}
-    );
+    return complete
+      ? this.getCompleteStyleDeclaration(lineIndex, charIndex)
+      : this._getStyleDeclaration(lineIndex, charIndex);
   }
 
   /**
@@ -246,14 +243,18 @@ export abstract class StyledText<
   }
 
   /**
-   * get the reference, not a clone, of the style object for a given character
+   * get the reference, not a clone, of the style object for a given character,
+   * if not style is set for a pre det
    * @param {Number} lineIndex
    * @param {Number} charIndex
-   * @return {Object} style object
+   * @return {Object} style object a REFERENCE to the existing one or a new empty object
    */
-  _getStyleDeclaration(lineIndex: number, charIndex: number) {
+  _getStyleDeclaration(
+    lineIndex: number,
+    charIndex: number
+  ): TextStyleDeclaration {
     const lineStyle = this.styles && this.styles[lineIndex];
-    return lineStyle ? lineStyle[charIndex] : null;
+    return lineStyle ? lineStyle[charIndex] ?? {} : {};
   }
 
   /**
@@ -263,12 +264,15 @@ export abstract class StyledText<
    * @param {Number} charIndex position of the character on the line
    * @return {Object} style object
    */
-  getCompleteStyleDeclaration(lineIndex: number, charIndex: number) {
+  getCompleteStyleDeclaration(
+    lineIndex: number,
+    charIndex: number
+  ): CompleteTextStyleDeclaration {
     return {
       // @ts-expect-error readonly
       ...pick(this, (this.constructor as typeof StyledText)._styleProperties),
-      ...(this._getStyleDeclaration(lineIndex, charIndex) || {}),
-    } as TextStyleDeclaration;
+      ...this._getStyleDeclaration(lineIndex, charIndex),
+    } as CompleteTextStyleDeclaration;
   }
 
   /**