pnp/cli-microsoft365

Add Eslint rule to handle promises

milanholemans opened this issue · 3 comments

Contributors and maintainers are humans and they make errors. Sometimes we merge stuff that we overlooked and have to fix later on. One thing I noticed is that we sometimes forget that logging a message is a promise. It used to be a void function, but since September last year, it has become a promise.
Sometimes messages are logged without being awaited, which results in undesired behavior. In fact, in almost all cases, we should always handle promises correctly. This is something that can be easily fixed by enabling an ESlint rule.

Goal

Configure ESlint rules to ensure that all promises are handled correctly.

Info

Something we can look into is the ESlint rule no-floating-promises. Let's check if this satisfies our needs.

Maybe it's also worth checking the no-misused-promises rule as extra.

Good one, @milanholemans! That's something that can be easily overlooked, so an ESLint rule for this could go a long way.

Good point! I vaguely recall looking at it but dismissing it at the end. Unfortunately, I don't know exactly why. My gut feeling tells me something wasn't working as expected, but let's double check. Maybe things have changed and we can cover this issue just fine now.

Just quickly jumping in here, as we are still in the process of merging #5837 , in which there were quite a few floating promises (mostly in the Command.ts file, where many logs werent being awaited), so could be handy if we wait until this PR is merged to avoid conflicts.