Adding TS to rollup #8019
Adding TS to rollup #8019
Conversation
|
@asturur you should write about this in the tracker issue |
| export type TDegree = Nominal<number, Degree>; | ||
| export type TRadian = Nominal<number, Radian>; |
There was a problem hiding this comment.
This is soething i have seen doing to other devs, is like a away to have 2 numbers that do not mix together.
This is good for radians vs degree and little else probably in our case.
There was a problem hiding this comment.
I looked at the angle=-angle thing
As you can see if I call cos(number) typescript will mark it as well unless I typecast it
The Nominal type needs more work.
There was a problem hiding this comment.
If I add an optional mark to the nominalTag 304767d it accepts number or TRadian but not TDegree:
There was a problem hiding this comment.
oh that would be fine, important is that they don't mix when they come out of a function that specifically outputs either radians or degree, i'll make that change.
Typescript should have this partitioned number somehow, even if is not a specific TS use case, but is a nice to have functionality.
| export const cos = (angle: TRadian): number => { | ||
| if (angle === 0) { return 1; } | ||
| var angleSlice = Math.abs(angle) / halfPI; | ||
| switch (angleSlice) { | ||
| case 1: case 3: return 0; | ||
| case 2: return -1; | ||
| } | ||
| return Math.cos(angle); | ||
| }; |
There was a problem hiding this comment.
i like the idea of one function per file and a file of constants.
I m also fine with a file grouping for example cos, sin, degreeToRadians and radiansToDegree, but not much else.
But since rollup has zero footprint on file numbers for me a function per file gives me easy accessibility to code, i can find the function both in the search and in the file search and i don't have to scroll files of thousands of lines searching the utils.
There was a problem hiding this comment.
I don't mind if a file for each or grouping.
We should then put cos, sin ,rad/deg utils into a seprate folder IMO called trig or something
There was a problem hiding this comment.
BTW, a code styling question, I don't have an opinion.
export const cos vs export function cos, why do you prefer the first?
This will make fabric so clean. I am very excited
There was a problem hiding this comment.
For top leveI functions is just an aestetic preference, i m not sure if there is a meaningful difference in functionality or speed. If there was i think it would be well chatted on the internet.
|
ok this works, tests pass, i add it to the general build process pr. |
Didn't test yet, just push up for visibility