ci(): Add linting #8024
ci(): Add linting #8024
Conversation
asturur
commented
Jun 25, 2022
•
asturur
commented
- update eslint to latest to support the typescript
- create a new command and configuration to lint ts files in order to slowly eliminate the old one
- pickup the default configuration ( that we probably will going to slighty change depending on preferences )
- fix the necessary changes in the test files in order for the new lint to pass
| @@ -12,7 +12,7 @@ import { halfPI } from '../constants'; | |||
| */ | |||
| export const cos = (angle: TRadian): number => { | |||
| if (angle === 0) { return 1; } | |||
| var angleSlice = Math.abs(angle) / halfPI; | |||
| const angleSlice = Math.abs(angle) / halfPI; | |||
There was a problem hiding this comment.
first lint request, at leat it works.
| @@ -1,5 +1,6 @@ | |||
| (function (global) { | |||
| var fabric = global.fabric; | |||
| /* this file needs to go away, cross browser style support is not fabricjs domain. | |||
There was a problem hiding this comment.
i didn't know we had this, but we need to remove it. Is probably for old days.
Safari eventuall doesn't support yet opacity in stile, meaning that opacity in style shouldn't be used imho.
There was a problem hiding this comment.
I am shocked that opacity isn't supported... WHY?
Well, how does it affect fabric?
There was a problem hiding this comment.
it doesn't. I think that if you pass a style object to the canvas, we transfer it on the container using this setStyle function.
And if for some weird reason you want your canvas with the opacity style property.
But really is not our problem to fix.
| @@ -35,7 +36,7 @@ | |||
| var parseEl = fabric.document.createElement('div'), | |||
| supportsOpacity = typeof parseEl.style.opacity === 'string', | |||
| supportsFilters = typeof parseEl.style.filter === 'string', | |||
| reOpacity = /alpha\s*\(\s*opacity\s*=\s*([^\)]+)\)/, | |||
| reOpacity = /alpha\s*\(\s*opacity\s*=\s*([^)]+)\)/, | |||
There was a problem hiding this comment.
another lint caught thing
There was a problem hiding this comment.
It seems that linting now works for us
There was a problem hiding this comment.
it was working even before, but our lint config lacked lint-recommended plugin, and had just the old formatting rules.
|
@ShaMan123 @melchiar have a look if you see something noteworthy. |
| @@ -10,6 +10,18 @@ | |||
| "pixelmatch": true | |||
There was a problem hiding this comment.
can't we use the eslint file for tests as well or extend it somehow?
Because globals should extend src globals, shouldn't they?
There was a problem hiding this comment.
it was setup like this ages ago, in theory we want to add to the normal config a bunch of globals for qunit and nothing else, but that requires extensive reformatting of tests files.
But yes we want extend the tests config from the source configuration
