react-play
react-play copied to clipboard
Fix 🐛 Bug #479: The Console has bunch of warnings
First thing, PLEASE READ THIS: ReactPlay Code Review Checklist
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
I worked on fixing issue #479
Fixes #479
Type of change
Please delete options that are not relevant.
- [x] Bug fix (non-breaking change which fixes an issue)
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged and published in downstream modules
The latest updates on your projects. Learn more about Vercel for Git ↗︎
Name | Status | Preview | Updated |
---|---|---|---|
react-play | ✅ Ready (Inspect) | Visit Preview | Aug 30, 2022 at 7:44AM (UTC) |
Crawler error
Crawler error
Yep, very surprisingly, it never passes.
@Sachin-chaurasiya Any idea on it? I am clueless why this branch alone fails to build!
@Sachin-chaurasiya Any idea on it? I am clueless why this branch alone fails to build!
Hey @atapas , I will have a look and if not working then will close this PR and open a new PR with the changes.
@Sachin-chaurasiya Any idea on it? I am clueless why this branch alone fails to build!
Hey @atapas , I will have a look and if not working then will close this PR and open a new PR with the changes.
It might be because of the dependency array. I can see the quoteArray is using a spread
operator so its always a new instance. @Sachin-chaurasiya can you just remove the dependency and push once
@Sachin-chaurasiya Any idea on it? I am clueless why this branch alone fails to build!
Hey @atapas , I will have a look and if not working then will close this PR and open a new PR with the changes.
It might be because of the dependency array. I can see the quoteArray is using a
spread
operator so its always a new instance. @Sachin-chaurasiya can you just remove the dependency and push once
@koustov , quoteArray
is being used in that effect so we should have that in dependency right? it does not matter how we are using the quoteArray
, only thing matter is what variable we are using for that effect.
If you want I can remove that but eslint will be complaining :)
quoteArray
is being used in that effect so we should have that in dependency right?
I didn't try by myself however looking at the error (time out) I can only suspect something is blocking the crawler from passing through the component. It's my educated guess that the quotearray might be the issue. So what I am suggesting is, to remove the dependency and push a commit, if the crawler passes we know what was wrong and then we can take appropriate measurements before getting the PR merge. So, it's just a TRY, nothing more than that
quoteArray
is being used in that effect so we should have that in dependency right?I didn't try by myself however looking at the error (time out) I can only suspect something is blocking the crawler from passing through the component. It's my educated guess that the quotearray might be the issue. So what I am suggesting is, to remove the dependency and push a commit, if the crawler passes we know what was wrong and then we can take appropriate measurements before getting the PR merge. So, it's just a TRY, nothing more than that
Sure, I will try that.
Hey @koustov , Thanks man, after removing quoteArray
from dependency it's working.
@atapas , Waiting for your review.