Skip to content

Fix enforce-store-naming-convention rule#119

Merged
igorkamyshev merged 3 commits intoeffector:masterfrom
iposokhin:fix-rule-enforce-store-naming-convention
Sep 5, 2022
Merged

Fix enforce-store-naming-convention rule#119
igorkamyshev merged 3 commits intoeffector:masterfrom
iposokhin:fix-rule-enforce-store-naming-convention

Conversation

@iposokhin
Copy link
Contributor

Current enforce-store-naming-convention rule passes this case, but it is not correct:

export const sum = createStore(0)
  .on(add, (sum) => sum + 1)
  .on(sub, (sum) => sum - 1)
  .reset(reset);

Same for the domain use case:

const d = createDomain();

const some = d.createStore(0)
  .on(add, (sum) => sum + 1)
  .on(sub, (sum) => sum - 1)
  .reset(reset);

This PR fixes this incorrect behavior

Copy link
Member

@igorkamyshev igorkamyshev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot 🙏

In my mind, it would be nice to have test-cases to check the correctness of the fix. Could you add it, please?

@sergeysova sergeysova changed the title Fix enforce-store-naming-convention rule Fix enforce-store-naming-convention rule Aug 30, 2022
@sergeysova sergeysova added the fix label Aug 30, 2022
@iposokhin iposokhin requested a review from igorkamyshev August 30, 2022 20:15
@igorkamyshev
Copy link
Member

Thanks for tests 🙏

I've found out that in this rule, previously written tests are stored in subdirectories prefix and suffix for different conventions.

Should we move this cases to the same files and extend case on suffix notation as well?

Copy link
Member

@igorkamyshev igorkamyshev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing, thanks!

Will be released in an hour 💙

@igorkamyshev igorkamyshev merged commit 9e11c86 into effector:master Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants