Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rule: no-forward #69

Closed
Kelin2025 opened this issue Oct 26, 2021 · 6 comments · Fixed by #82
Closed

Rule: no-forward #69

Kelin2025 opened this issue Oct 26, 2021 · 6 comments · Fixed by #82
Labels
enhancement New feature or request

Comments

@Kelin2025
Copy link
Member

I'd like to ban forward({ from, to }) operator, and replace it with sample({ clock, target }) instead
The reason is that sample does the same thing that forward does, but it's completely superior in terms of features
I figured out that every time I write forward somewhere, after a ~hour I rewrite it to sample because OK now I need to transform my data
The only problem is that forward has different execution priority rather than sample, so sometimes it might be important

@AlexandrHoroshih
Copy link
Member

The only problem is that forward has different execution priority rather than sample, so sometimes it might be important

sample with greedy: true has equal priority with forward:
https://share.effector.dev/nGbDKALC

Should rule recommend to use "greedy" version of sample instead of forward?
In most situations there would be no difference between all of them, no matter if sample is greedy or not 🤔

If we could somehow define specific situations when it does matter, then rule could provide better hint on which kind of sample to use 🤔

@igorkamyshev
Copy link
Member

In my opinion, differences in priority isn't important in 99% cases. I think, if you rely on them, you know it.

@AlexandrHoroshih
Copy link
Member

AlexandrHoroshih commented Oct 26, 2021

In my opinion, differences in priority isn't important in 99% cases. I think, if you rely on them, you know it.

I agree, but i think, that this rule can backfire, if this plugin is added to a project, that was developed for some time without it and there is a case when priority matters

It can go like this: user will change forward -> sample according to the rule and then will catch an observable change in behavior of the app, which, for sure, will be confusing

I think, hint of the rule should at least mention that there is a such possibility

@igorkamyshev
Copy link
Member

Good point

@Kelin2025
Copy link
Member Author

After all, if there's a really-really important case where you should use forward, you can always // eslint-disable-next-line effector/no-forward

@igorkamyshev igorkamyshev added the enhancement New feature or request label Nov 3, 2021
@igorkamyshev
Copy link
Member

So, I've decided to add this rule to plugin, but do not include it to recommended preset because it can affect program results.

@igorkamyshev igorkamyshev mentioned this issue Dec 25, 2021
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants