salesforce / eslint-plugin-lwc

Official ESLint rules for LWC
MIT License
98 stars 32 forks source link

@W-15205338 adding new rule for unguarded async ops and eventing #166

Closed abhagta-sfdc closed 1 month ago

abhagta-sfdc commented 1 month ago

@lpomerleau here is the log for lint scan in es-content-authoring repo with no-unguarded-async-event enabled [lint.log](https://github.com/user-attachments/files/17469921/lint.log)

pmdartus commented 1 month ago

Thanks for sharing the ESLint logs @abhagta-sfdc.

Looking at the results, in it's current shape the @lwc/lwc/ssr-no-unguarded-async-event rule creates too many false positive. Here are a couple of examples from the log containing 394 warnings:

I wouldn't recommend engineers to adopt this rule. Requiring to wrap all async code and event dispatching in import.meta.env.SSR checks will result in worse quality and less maintainable code. This is the type of issue, I expect to be addressed through comprehensive testing unit and integration testing, not through static analysis.

abhagta-sfdc commented 1 month ago

@pmdartus Thank you for the review! As per the description in the work item assigned to us, we were supposed to flag all async/await or event operations. Your clarification is helpful and much appreciated. @lpomerleau Just checking to confirm if we should close this PR(as PM's suggestion is to deal this via comprehensive testing unit and integration testing)

pmdartus commented 1 month ago

Please don't close the PR for now @abhagta-sfdc. This is my take on those rules, but I am not the decision maker here.

lpomerleau commented 1 month ago

I'm on board to close this PR. We have public and internal doc which discourages the use of async operations. Additionally, async code does not break SSR; it just doesn't get fully executed, so it is still technically portable.