You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@devlake.apache.org by GitBox <gi...@apache.org> on 2022/10/08 16:23:16 UTC
[GitHub] [incubator-devlake] likyh opened a new issue, #3334: [Refactor][Module Name] Refactor title
likyh opened a new issue, #3334:
URL: https://github.com/apache/incubator-devlake/issues/3334
## What and why to refactor
https://github.com/apache/incubator-devlake/blob/7b0199a6d8557d4f18b73200d619a96d10df0163/config-ui/src/pages/blueprints/create-blueprint.jsx#L501-L505
delete set function in deps.
Don't add the set functions in deps unless they are necessary by some detail reason. Delete them will not trigger the linter's warning.
An example:
In one file:
```js
const [a, setA] = useState(..)
const updateColumnInA = useCallback(
(key, value) => (setA[key] = value),
[setA]
)
```
In another fileļ¼
```
const [b, setB] = useState(..)
useEffect(() => {
updateColumnInA('xxxxx', b)
}, [b, updateColumnInA])
```
Once a changed, b will be updated. It is not necessary and sometimes it will lead to an endless loop.
## Describe the solution you'd like
check all deps and try to delete them depending on the set function unless they are necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on issue #3334: [Refactor][config-ui] try to delete set function in deps.
Posted by GitBox <gi...@apache.org>.
e2corporation commented on issue #3334:
URL: https://github.com/apache/incubator-devlake/issues/3334#issuecomment-1273272049
@likyh I'm not saying that you can't optimize an Effect or fix a potential issue you have strongly identified, but the task of "delete all set functions in deps" seems a bit of a generalization. Are you trying to do this to the entire codebase or just this one `handleTransformationCancel` method ?
If you are just refactoring `handleTransformationCancel` then go for it, If you are trying to impose this observation of no-setters in deps on all files that's a different issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] likyh commented on issue #3334: [Refactor][config-ui] try to delete set function in deps.
Posted by GitBox <gi...@apache.org>.
likyh commented on issue #3334:
URL: https://github.com/apache/incubator-devlake/issues/3334#issuecomment-1273229713
@e2corporation I describe a scenario that may lead to an endless loop, rather than saying that there is an endless loop now. In fact, the problem also exists. I remember seeing in the object that you annotated the original simple scheme and used a complex scheme to avoid an endless loop.
In any case, this is a potential problem. It should not be said that if there is no problem, it will not be solved.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on issue #3334: [Refactor][config-ui] try to delete set function in deps.
Posted by GitBox <gi...@apache.org>.
e2corporation commented on issue #3334:
URL: https://github.com/apache/incubator-devlake/issues/3334#issuecomment-1273204128
@likyh The goal should be to embrace proper usage of Effects and understand dependency management, not move all properties as callback arguments so as to _avoid_ the linter warnigs? Just having a setter as a dependency is not the problem that causes endless loop.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation closed issue #3334: [Refactor][config-ui] try to delete set function in deps.
Posted by GitBox <gi...@apache.org>.
e2corporation closed issue #3334: [Refactor][config-ui] try to delete set function in deps.
URL: https://github.com/apache/incubator-devlake/issues/3334
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on issue #3334: [Refactor][config-ui] try to delete set function in deps.
Posted by GitBox <gi...@apache.org>.
e2corporation commented on issue #3334:
URL: https://github.com/apache/incubator-devlake/issues/3334#issuecomment-1273207699
@likyh We don't have any loops to worry about last time I checked, except for a recent PR by Junren on DataTransformations which is the only warning on the lint log currently.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on issue #3334: [Refactor][config-ui] try to delete set function in deps.
Posted by GitBox <gi...@apache.org>.
e2corporation commented on issue #3334:
URL: https://github.com/apache/incubator-devlake/issues/3334#issuecomment-1273244581
@likyh I think a majority of the problem here is your interpretation of how Effects should work. There are times when a func will have callback args, and other times where a Setter is not needed as a dependency but that depends on the _context_ of how the effect is being used and how the developer is implementing the flow.
If you encounter a loop problem going forward, you simply troubleshoot it and fix related dependencies to resolve the issue. If adding a required dependency causes loop, then look closer at the dependency itself whether it's a setter func or some other generic constant that simply needs a callback wrap to solve the problem.
The comment about using a an Object prop rather than the whole object is well documented by react, and also by the article I shared. I mentioned that to give you a better understanding of nuances when using Objects as a dependency. In some cases we want the whole object, in other cases we only need a single prop like `Object?.id`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] likyh commented on issue #3334: [Refactor][config-ui] try to delete set function in deps.
Posted by GitBox <gi...@apache.org>.
likyh commented on issue #3334:
URL: https://github.com/apache/incubator-devlake/issues/3334#issuecomment-1273206525
@e2corporation Please read the issue again. That is the solution for some endless loop situations. Not for linter waring.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org