Fomantic-UI icon indicating copy to clipboard operation
Fomantic-UI copied to clipboard

Unify/modernize code with eslint - use let, drop Node 12 support

Open mvorisek opened this issue 2 years ago • 4 comments

let vs var declaration is safer, see https://stackoverflow.com/questions/762011/what-is-the-difference-between-let-and-var and is supported by every modern browser incl. IE11 - https://caniuse.com/?search=let

also Node 12 support is dropped as no longer supported by eslint-plugin-unicorn dep and it is EOL

also use rgb(... x%) instead of rgba(...)

mvorisek avatar Dec 04 '22 12:12 mvorisek

@lubber-de also what are your thoughts on using even const when the variables are unchanged?

mvorisek avatar Dec 04 '22 20:12 mvorisek

PR is done.

Most of the changes are done with autofix, so they can be reviewed by cherrypicking the .eslintrc.js config (and the added yarn deps) onto develop & run eslint autofix. The difference are manual changes.

mvorisek avatar Dec 05 '22 12:12 mvorisek

Thanks. This has to wait until 2.10.x for the following reasons (i haven't reviewed yet!):

  • IE11 does not completely support let when used inside for loops afaik (and caniuse states that)
  • Dropping Node 12 support
  • Does not work correctly in Safari < 11 , Android 4.4 anymore (yes, we officially do not support them anymore, but we have at least currently stated in the readme, that FUI will basically still work)
  • Deep testing is needed, i suspect non-obvious bugs

However, i'll try to review as quick as possible 😉

lubber-de avatar Dec 05 '22 12:12 lubber-de

@lubber-de also what are your thoughts on using even const when the variables are unchanged?

Fine for me. However: Same reasons as for let apply as mentioned above

lubber-de avatar Dec 05 '22 12:12 lubber-de