Skip to content

Commit

Permalink
chore(TS): Add more type-checks for remaining files (#9097)
Browse files Browse the repository at this point in the history
  • Loading branch information
@asturur
asturur authored Jul 23, 2023
1 parent 1d3d80f commit b24ca42
Show file tree
Hide file tree
Showing 29 changed files with 465 additions and 320 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@

## [next]

- chore(TS): Add type-checking to files excluded with ts-nocheck [#9097](https://github.com/fabricjs/fabric.js/pull/9097)
- chore(TS): Add type-checking to files excluded with ts-nocheck ( Parser mostly ) [#9085](https://github.com/fabricjs/fabric.js/pull/9085)
- docs(): revise test section [#9114](https://github.com/fabricjs/fabric.js/pull/9114)
- fix(): #8344 stroke projection [#8374](https://github.com/fabricjs/fabric.js/pull/8374)
- fix(Filters) Removing type from the options passed in the constructor [#9089](https://github.com/fabricjs/fabric.js/pull/9089)
- feat(InteractiveObject): add `getActiveControl()` to expose `__corner` [#9102](https://github.com/fabricjs/fabric.js/pull/9102)
- ci(sandbox): bump next.js [#9100](https://github.com/fabricjs/fabric.js/pull/9100)
- test(playwright): add snapshots, refactor utils, coverage [#9078](https://github.com/fabricjs/fabric.js/pull/9078)
- chore(TS) Add type-checking to files excluded with ts-nocheck ( Parser mostly ) [#9085](https://github.com/fabricjs/fabric.js/pull/9085)
- test(Text): Add some tests for text in Jest [#9083](https://github.com/fabricjs/fabric.js/pull/9083)
- ci(): Install system deps only when necessary [#9086](https://github.com/fabricjs/fabric.js/pull/9086)
- fix(util, Path): path distance measurement fix for M cmd [#9076](https://github.com/fabricjs/fabric.js/pull/9076)
Expand Down
16 changes: 8 additions & 8 deletions CONTRIBUTING.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,15 @@ It is more than likely you will be requested to change stuff and refine your wor
[![🧪](../../actions/workflows/tests.yml/badge.svg)](../../actions/workflows/tests.yml)
[![CodeQL](../../actions/workflows/codeql-analysis.yml/badge.svg)](../../actions/workflows/codeql-analysis.yml)

| Suite | unit (node) | e2e (browser) |
| ------------------------------------------------------------------------ | :----------------------------------------------------------------------------------------------- | :------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| Framework | [`jest`][jest] | [`playwright`][playwright] |
| Setup | | <pre>npm run build -- -f -w</pre> |
| Suite | unit (node) | e2e (browser) |
| ---------------------------------------------------------------------------- | :------------------------------------------------------------------------------------------------------- | :------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| Framework | [`jest`][jest] | [`playwright`][playwright] |
| Setup | | <pre>npm run build -- -f -w</pre> |
| Running Tests<br><br><br><br><br><pre>\<test cmd\> -- [filter] [watch]</pre> | <pre>npm run test:jest -- [filters] [-w]</pre><br><br><br>It is advised to use filters to save time.<br> | <pre>npm run test:e2e -- [filters] [--ui]</pre><br>In some machines babel is flaky and doesn't build the test files. In that is the case, try running the command using `npx` directly, see [`playwright.setup.ts`](./playwright.setup.ts). |
| Writing Tests | Add/update `src/*.(spec\|test).ts` files | - Update tests in `e2e/tests`<br>- Create a new test based on `e2e/template` |
| Test Gen | | <pre>npm start vanilla<br>npx playwright codegen http://localhost:1234</pre> |
| Test Spec | | - `index.ts`: built and loaded into the web app<br> - `index.spec.ts`: test spec<br> |
| Outputs | Snapshots next to the test file | - Snapshots next to the test file <br>- `e2e/test-report`<br>- `e2e/test-results` |
| Writing Tests | Add/update `src/*.(spec\|test).ts` files | - Update tests in `e2e/tests`<br>- Create a new test based on `e2e/template` |
| Test Gen | | <pre>npm start vanilla<br>npx playwright codegen http://localhost:1234</pre> |
| Test Spec | | - `index.ts`: built and loaded into the web app<br> - `index.spec.ts`: test spec<br> |
| Outputs | Snapshots next to the test file | - Snapshots next to the test file <br>- `e2e/test-report`<br>- `e2e/test-results` |

### Legacy Test Suite

Expand Down
12 changes: 11 additions & 1 deletion fabric.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,17 @@ export { createCollectionMixin } from './src/Collection';

export * as util from './src/util';

export * from './src/parser';
export { loadSVGFromString } from './src/parser/loadSVGFromString';
export { loadSVGFromURL } from './src/parser/loadSVGFromURL';
export { parseSVGDocument } from './src/parser/parseSVGDocument';

// todo convert tests to jest and stop exporting those.
export { parseAttributes } from './src/parser/parseAttributes';
export { parseStyleAttribute } from './src/parser/parseStyleAttribute';
export { parsePointsAttribute } from './src/parser/parsePointsAttribute';
export { parseTransformAttribute } from './src/parser/parseTransformAttribute';
export { getCSSRules } from './src/parser/getCSSRules';
export { parseFontDeclaration } from './src/parser/parseFontDeclaration';

export { Control } from './src/controls/Control';
export * as controlsUtils from './src/controls';
Expand Down
7 changes: 3 additions & 4 deletions src/CommonMethods.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
//@ts-nocheck
import { Observable } from './Observable';

export class CommonMethods<EventSpec> extends Observable<EventSpec> {
Expand Down Expand Up @@ -37,7 +36,7 @@ export class CommonMethods<EventSpec> extends Observable<EventSpec> {
}

_set(key: string, value: any) {
this[key] = value;
this[key as keyof this] = value;
}

/**
Expand All @@ -57,7 +56,7 @@ export class CommonMethods<EventSpec> extends Observable<EventSpec> {
* @param {String} property Property name
* @return {*} value of a property
*/
get(property: string) {
return this[property];
get(property: string): any {
return this[property as keyof this];
}
}
9 changes: 3 additions & 6 deletions src/gradient/Gradient.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,9 @@ import { uid } from '../util/internals/uid';
import { pick } from '../util/misc/pick';
import { matrixToSVG } from '../util/misc/svgParsing';
import { linearDefaultCoords, radialDefaultCoords } from './constants';
import {
parseColorStops,
parseCoords,
parseGradientUnits,
parseType,
} from './parser';
import { parseColorStops } from './parser/parseColorStops';
import { parseCoords } from './parser/parseCoords';
import { parseType, parseGradientUnits } from './parser/misc';
import type {
ColorStop,
GradientCoords,
Expand Down
3 changes: 0 additions & 3 deletions src/gradient/parser/index.ts
View file

This file was deleted.

62 changes: 39 additions & 23 deletions src/parser/applyViewboxTransform.ts
View file
Original file line number Diff line number Diff line change
@@ -1,20 +1,30 @@
// @ts-nocheck
import { svgNS } from './constants';
import {
parsePreserveAspectRatioAttribute,
parseUnit,
} from '../util/misc/svgParsing';
import { svgViewBoxElementsRegEx, reViewBoxAttrValue } from './constants';
import { NONE } from '../constants';

export type ParsedViewboxTransform = Partial<{
width: number;
height: number;
minX: number;
minY: number;
viewBoxWidth: number;
viewBoxHeight: number;
}>;

/**
* Add a <g> element that envelop all child elements and makes the viewbox transformMatrix descend on all elements
*/

export function applyViewboxTransform(element: HTMLElement) {
export function applyViewboxTransform(
element: Element
): ParsedViewboxTransform {
if (!svgViewBoxElementsRegEx.test(element.nodeName)) {
return {};
}
let viewBoxAttr = element.getAttribute('viewBox');
const viewBoxAttr: string | null = element.getAttribute('viewBox');
let scaleX = 1;
let scaleY = 1;
let minX = 0;
Expand All @@ -25,49 +35,54 @@ export function applyViewboxTransform(element: HTMLElement) {
const heightAttr = element.getAttribute('height');
const x = element.getAttribute('x') || 0;
const y = element.getAttribute('y') || 0;
const missingViewBox =
!viewBoxAttr || !(viewBoxAttr = viewBoxAttr.match(reViewBoxAttrValue));
const goodViewbox = viewBoxAttr && reViewBoxAttrValue.test(viewBoxAttr);
const missingViewBox = !goodViewbox;
const missingDimAttr =
!widthAttr || !heightAttr || widthAttr === '100%' || heightAttr === '100%';
const toBeParsed = missingViewBox && missingDimAttr;
const parsedDim: any = {};

let translateMatrix = '';
let widthDiff = 0;
let heightDiff = 0;

parsedDim.width = 0;
parsedDim.height = 0;
parsedDim.toBeParsed = toBeParsed;

if (missingViewBox) {
if (
(x || y) &&
element.parentNode &&
element.parentNode.nodeName !== '#document'
) {
translateMatrix =
' translate(' + parseUnit(x) + ' ' + parseUnit(y) + ') ';
' translate(' + parseUnit(x || '0') + ' ' + parseUnit(y || '0') + ') ';
matrix = (element.getAttribute('transform') || '') + translateMatrix;
element.setAttribute('transform', matrix);
element.removeAttribute('x');
element.removeAttribute('y');
}
}

if (toBeParsed) {
return parsedDim;
if (missingViewBox && missingDimAttr) {
return {
width: 0,
height: 0,
};
}

const parsedDim: ParsedViewboxTransform = {
width: 0,
height: 0,
};

if (missingViewBox) {
parsedDim.width = parseUnit(widthAttr);
parsedDim.height = parseUnit(heightAttr);
parsedDim.width = parseUnit(widthAttr!);
parsedDim.height = parseUnit(heightAttr!);
// set a transform for elements that have x y and are inner(only) SVGs
return parsedDim;
}
minX = -parseFloat(viewBoxAttr[1]);
minY = -parseFloat(viewBoxAttr[2]);
const viewBoxWidth = parseFloat(viewBoxAttr[3]);
const viewBoxHeight = parseFloat(viewBoxAttr[4]);

const pasedViewBox = viewBoxAttr.match(reViewBoxAttrValue)!;
minX = -parseFloat(pasedViewBox[1]);
minY = -parseFloat(pasedViewBox[2]);
const viewBoxWidth = parseFloat(pasedViewBox[3]);
const viewBoxHeight = parseFloat(pasedViewBox[4]);
parsedDim.minX = minX;
parsedDim.minY = minY;
parsedDim.viewBoxWidth = viewBoxWidth;
Expand Down Expand Up @@ -122,8 +137,9 @@ export function applyViewboxTransform(element: HTMLElement) {
) {
return parsedDim;
}
if ((x || y) && element.parentNode.nodeName !== '#document') {
translateMatrix = ' translate(' + parseUnit(x) + ' ' + parseUnit(y) + ') ';
if ((x || y) && element.parentNode!.nodeName !== '#document') {
translateMatrix =
' translate(' + parseUnit(x || '0') + ' ' + parseUnit(y || '0') + ') ';
}

matrix =
Expand Down
4 changes: 0 additions & 4 deletions src/parser/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,6 @@ export const svgValidTagNames = [
'vector-effect': 'strokeUniform',
'image-rendering': 'imageSmoothing',
},
colorAttributes = {
stroke: 'strokeOpacity',
fill: 'fillOpacity',
},
fSize = 'font-size',
cPath = 'clip-path';

Expand Down
Loading

0 comments on commit b24ca42

Please sign in to comment.