chore(ts): WIP - Convert utils #8123

asturur · 2022-08-04T21:45:06Z

This will be a partial conversion of utils, i will stop at the misc file and review/merge.
All together is a mess, but luckily we can also swap the imports at that point.

ShaMan123 · 2022-08-05T07:53:34Z

src/util/misc/radiansDegreesConversion.ts

+ * @param {TRadian} radians value in radians
+ * @return {TDegree} value in degrees
+ */
+export const radiansToDegrees = (radians: TRadian): TDegree => (radians / PiBy180) as TDegree;


ShaMan123 · 2022-08-05T07:56:14Z

src/util/misc/sin.ts

+  let value = 1;
+  if (angle < 0) {
+    // sin(-a) = -sin(a)
+    value = -1;


make a const?

Suggested change

value = -1;

value = Math.sign(angle);

i always forget about Math.sign

asturur · 2022-08-08T09:01:42Z

i know what i broke, i ll try to fix it today.
I messed up the imports/exports

asturur · 2022-08-10T10:23:19Z

@ShaMan123 i had to revert a couple of imports.
The color class importing some util and the animate color util importing the color class was creating a circular dependency.

asturur · 2022-08-10T10:26:10Z

src/util/internals/removeFromArray.ts

+ * @param {*} value
+ * @return {Array} original array
+ */
+export const removeFromArray = (array: any[], value: any): any[] => {


i m sure there is a way to tell Typescript that the returned array is the same type of the input array

Generic types

Suggested change

export const removeFromArray = (array: any[], value: any): any[] => {

export function removeFromArray<T>(array: T[], value: T): T[] {

and of course T can be a mix of types. ( even if in our case often isn't )

fat arrows aren't good for TS as functions?

asturur · 2022-08-10T10:57:04Z

Ok something broke with the import system and i had to patch it. I have no idea why.

ShaMan123

Regarding the circular import
I have seen it as well and managed to fix it in a different way without reverting
Try importing utils directly and not from the util index OR make animate_color or whatever a module (even as a hack).
That should fix it.
rollup seems to be a decent tool

ShaMan123 · 2022-08-10T14:41:44Z

src/mixins/observable.mixin.ts

@@ -1,6 +1,7 @@
 //@ts-nocheck
 (function(global) {
  var fabric = global.fabric;
+  console.log(fabric)


yeah i was investigating why fabric was undefined

This reverts commit 9a79f9c.

prevent import cycle

github-actions · 2022-08-10T15:33:18Z

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   82.06 |    74.54 |   84.42 |   80.66 |                                               
 fabric.js |   82.06 |    74.54 |   84.42 |   80.66 | ...,27459,27517,27527-27571,27679,27766,27970 
-----------|---------|----------|---------|---------|-----------------------------------------------

ShaMan123 · 2022-08-10T16:35:50Z

Can't be generic i thinl בתאריך יום ד׳, 10 באוג׳ 2022, 19:28, מאת Andrea Bogazzi ‏< ***@***.***>:

…

***@***.**** commented on this pull request. ------------------------------ In src/util/internals/removeFromArray.ts <#8123 (comment)>: > @@ -0,0 +1,16 @@ +/** + * Removes value from an array. + * Presence of value (and its position in an array) is determined via `Array.prototype.indexOf` + * @static + * @memberof fabric.util + * @param {Array} array + * @param {*} value + * @return {Array} original array + */ +export const removeFromArray = (array: any[], value: any): any[] => { fat arrows aren't good for TS as functions? — Reply to this email directly, view it on GitHub <#8123 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIGAW4PO32BB4AWGBPQ74HDVYPKC7ANCNFSM55UAKANQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

asturur · 2022-08-10T16:36:18Z

src/util/misc/vectors.ts

+ * @param {Point} to
+ * @returns {Point} vector
+ */
+export const createVector = (from: IPoint, to: IPoint): Point => new Point(to).subtract(from);


i don't get this. if the type was Point , way we changed it to interface? we expect devs to use points or objects with x/y?

You are right
I committed it and felt the same
Reverr this
And safeguard createVector instead

asturur · 2022-08-11T15:38:20Z

note for later self: const foo = <T,>(x: T) => x;

github-actions · 2022-08-14T22:03:17Z

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   82.06 |    74.54 |   84.42 |   80.66 |                                               
 fabric.js |   82.06 |    74.54 |   84.42 |   80.66 | ...,27459,27517,27527-27571,27679,27766,27970 
-----------|---------|----------|---------|---------|-----------------------------------------------

github-actions · 2022-08-14T22:03:55Z

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   82.05 |    74.51 |   84.42 |   80.64 |                                               
 fabric.js |   82.05 |    74.51 |   84.42 |   80.64 | ...,27458,27516,27526-27570,27678,27765,27969 
-----------|---------|----------|---------|---------|-----------------------------------------------

github-actions · 2022-08-14T22:30:41Z

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   82.11 |    74.57 |   84.42 |   80.71 |                                               
 fabric.js |   82.11 |    74.57 |   84.42 |   80.71 | ...,27458,27516,27526-27570,27678,27765,27969 
-----------|---------|----------|---------|---------|-----------------------------------------------

Co-authored-by: ShaMan123 <shacharnen@gmail.com>

some changes

29d6dcd

ShaMan123 reviewed Aug 5, 2022

View reviewed changes

asturur changed the title ~~chore(ts): Convert utils~~ chore(ts): WIP - Convert utils Aug 5, 2022

asturur added 8 commits August 7, 2022 23:51

use Math.sign

2a9fac7

more changes

121e784

fixed things

02c1259

work progress

87890f5

cleaner

f259393

small fixes

43ec71f

Merge branch 'master' into convert-utils-to-es6

922f2f1

fixed conflicts

74753c6

asturur added 3 commits August 10, 2022 11:32

try to restore imports

6777c10

not fixed

12e0d9e

works now

9a79f9c

asturur commented Aug 10, 2022

View reviewed changes

different index.js

538d1e7

ShaMan123 reviewed Aug 10, 2022

View reviewed changes

ShaMan123 added 4 commits August 10, 2022 17:59

Revert "works now"

e55fd6f

This reverts commit 9a79f9c.

expose array utils as module

008912f

prevent import cycle

remove falseFunction

b6eaa27

fix(): createVector

f66e696

asturur commented Aug 10, 2022

View reviewed changes

asturur added 2 commits August 15, 2022 00:00

typed the removeFromArray

32d4371

no console log

d86dc4d

asturur added 2 commits August 15, 2022 00:12

restore to point

16130e0

safeguard

e61b86b

asturur merged commit 93a7847 into master Aug 14, 2022

asturur deleted the convert-utils-to-es6 branch September 11, 2022 23:03

frankrousseau pushed a commit to cgwire/fabric.js that referenced this pull request Jan 6, 2023

chore(ts): Convert some utils (fabricjs#8123)

702a612

Co-authored-by: ShaMan123 <shacharnen@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(ts): WIP - Convert utils #8123

chore(ts): WIP - Convert utils #8123

asturur commented Aug 4, 2022 •

edited

Loading

ShaMan123 Aug 5, 2022

ShaMan123 Aug 5, 2022

asturur Aug 5, 2022

asturur commented Aug 8, 2022

asturur commented Aug 10, 2022

asturur Aug 10, 2022

ShaMan123 Aug 10, 2022

asturur Aug 10, 2022

asturur Aug 10, 2022

asturur commented Aug 10, 2022

ShaMan123 left a comment •

edited

Loading

ShaMan123 Aug 10, 2022

asturur Aug 10, 2022

github-actions bot commented Aug 10, 2022 •

edited

Loading

ShaMan123 commented Aug 10, 2022 via email

asturur Aug 10, 2022

ShaMan123 Aug 10, 2022

asturur commented Aug 11, 2022

github-actions bot commented Aug 14, 2022 •

edited

Loading

github-actions bot commented Aug 14, 2022 •

edited

Loading

github-actions bot commented Aug 14, 2022 •

edited

Loading

	export const removeFromArray = (array: any[], value: any): any[] => {
	export function removeFromArray<T>(array: T[], value: T): T[] {

chore(ts): WIP - Convert utils #8123

chore(ts): WIP - Convert utils #8123

Conversation

asturur commented Aug 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asturur commented Aug 8, 2022

asturur commented Aug 10, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asturur commented Aug 10, 2022

ShaMan123 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 10, 2022 • edited Loading

Code Coverage Summary

ShaMan123 commented Aug 10, 2022 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asturur commented Aug 11, 2022

github-actions bot commented Aug 14, 2022 • edited Loading

Code Coverage Summary

github-actions bot commented Aug 14, 2022 • edited Loading

Code Coverage Summary

github-actions bot commented Aug 14, 2022 • edited Loading

Code Coverage Summary

asturur commented Aug 4, 2022 •

edited

Loading

ShaMan123 left a comment •

edited

Loading

github-actions bot commented Aug 10, 2022 •

edited

Loading

github-actions bot commented Aug 14, 2022 •

edited

Loading

github-actions bot commented Aug 14, 2022 •

edited

Loading

github-actions bot commented Aug 14, 2022 •

edited

Loading