feat(eslint-plugin): prefer workflow events#16150
Conversation
🦋 Changeset detectedLatest commit: bc35642 The changes in this PR will be included in the next version bump. This PR includes changesets to release 79 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Thanks for the contribution! A few items need to be addressed before this can move forward: Community contribution adding a new ESLint rule ('prefer-workflow-events') to warn when module-level events have workflow-level equivalents, plus a new 'core' preset. PR totals 828 changed lines across 11 files with no linked GitHub issue, which is above the 500-line threshold requiring pre-approval per CONTRIBUTING.md. Additionally, adding new rules to the 'recommended' preset changes its behaviour for existing users and should be versioned as a minor bump in the changeset, not a patch.
Triggered by: new PR opened |
shahednasser
left a comment
There was a problem hiding this comment.
Thank you for your contribution @leobenzol ! I think this is a good addition, but would approach it differently:
- If the goal of
coreis to disable the rule for this repo, then I think we should modify the eslint config at the root. Would avoid causing confusion for end-users on when to use it. - The rule is too broad, and from an end user perspective I would focus on the subscription part as that is the part where users often get confused. Would change the rule to work only on subscriber files and detect the event name to be either the enum, or in the format
{domain}.{action}. Internal services on the other hand are{module}.{data-model}.{action}. For the enum, we show the warning if we know it's an internal service event enum, with an auto-fix action if possible. For the literal string, we show a warning that the event doesn't match workflow event enums and to confirm that the event is correct.
Summary
What — What changes are introduced in this PR?
Adds a new eslint rule to warn users if they use a module-level event that has a corresponding workflow-level event, and logic to fix it automatically.
Added a new "core" preset that doesn't include this kind of end-user-only rule
Why — Why are these changes relevant or necessary?
I got bit by this personally a while ago and it seems to be a frequent issue.
How — How have these changes been implemented?
Testing — How have these changes been tested, or how can the reviewer test the feature?
New tests + manual verification
Examples
Checklist
Please ensure the following before requesting a review:
yarn changesetand follow the prompts