Eslint airbnb transloadit

[mifi]

Use eslint-config-transloadit and fix issues uncovered.
See #90 and transloadit/eslint-config-transloadit#1

[kvz]

these seem unrelated 🤔

[mifi]

Yes. I discovered some issues/improvements while converting to the new eslint config. I can pull those commits out to another branch

[kvz]

No need as long as you are aware 👌

[kvz]

i like the sorting of requires

[kvz]

Few nitpicks/questions, it does look good overall!

One thing, there are quite some cases where you disable eslint rules the code.

I wonder, if we apply this to the API, if that will be crippling. Like. so many bailouts to add.

But i guess, we can bail out on a per-project level in the .eslintrc, until we move to a reality where all issues can be fixed / explicitly ignored

[mifi]

I can comment on the eslint-disables:

• no-await-in-loop: This rule is meant to prevent people from looping over a list of things in series, when it could be run much faster in parallell, with p-map etc. Where I disabled this is because I know I want it to run in series. See https://eslint.org/docs/rules/no-await-in-loop
• no-loop-func: making functions inside a loop is error prone, because if the user references variables in the loop from the function, they will not have the value that you expect. I agree this one is a bit strict and maybe we should disable it. See https://eslint.org/docs/rules/no-loop-func
• no-param-reassign: reassigning function arguments can lead to confusing behavior. I disabled here because the decorateError is intentionally not pure as it is meant to mutate the input. https://eslint.org/docs/rules/no-param-reassign
• no-bitwise was disabled because in this case we are actually doing a bitwise operation. However in normal JS programming using a bitwise operator is usually a mistake (a typo) which could cause unexpected errors.
• no-constant-condition prevents code like if (true), but here it also kicks on while.
• class-methods-use-this says that the function does not use anything from the class and should then be static or pulled out. but I was lazy to rewrite the unit tests so I wanted to keep it there (mockability)
• no-console is good to catch accidental console logs (a library shouldn't do this). But for examples and cli apps etc this can be disabled
• no-underscore-dangle is just a style from airbnb eslint that underscores are not to be used (but I disabled for instance method names).

I'm totally fine with completely disabling some or all of these in the central config also

[mifi]

I think if we introduce this style guide to the existing code, we can definitely selectively disable certain rules inside .eslintrc and keep that disable list as a todo for each project

[kvz]

Thanks for the detailed explanation. I think we should go with strict for now, loosen up in individual projects in their own .eslintrc, and based on feedback loosen up in a next version of eslint-transloadit.

Regarding all the extra safety it provides, i'm ok to have many changes. As for stylistic changes, not so much - but that also doesn't seem to be the case looking at this PR 👌

[kvz]

Feel free to merge this one already if you like 🎉

[kvz]

or ... do we first want to finalize work on our linting, and then merge all of the open prs .. 🤔 there may be changes based on findings in the other repos.

[mifi]

I think fixing the other repos is a much bigger task, so I think we can merge this one now. If we need to update the config in the future we can create a new PR here for the new version.