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.

Sign up for GitHub

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

Jump to bottom

fix: bad strategies setState #724

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: bad strategies setState #724

wants to merge 1 commit into from

Conversation

RobertHebel
Copy link
Contributor

@RobertHebel RobertHebel commented Jul 1, 2021 •
edited
Loading

Deployed to https://beghp.github.io/gh-pages-rc-5
- [x] an issue linked to the PR

It fixes: #710, #714 #707

@RobertHebel
Copy link
Contributor Author

Warnings
⚠️

MODULE : node_modules/valid-url
| LICENSE : MIT*
| LICENSE_FILE : node_modules/valid-url/LICENSE
| REPOSITORY: https://github.com/ogt/valid-url
| PUBLISHER : undefined
| EMAIL : undefined
| URL : undefined
⚠️

MODULE : node_modules/trim
| LICENSE : MIT*
| LICENSE_FILE : node_modules/trim/Readme.md
| REPOSITORY: undefined
| PUBLISHER : TJ Holowaychuk
| EMAIL : [email protected]
| URL : undefined
⚠️

MODULE : node_modules/sha
| LICENSE : (BSD-2-Clause OR MIT)
| LICENSE_FILE : node_modules/sha/LICENSE
| REPOSITORY: https://github.com/ForbesLindesay/sha
| PUBLISHER : undefined
| EMAIL : undefined
| URL : undefined
⚠️

MODULE : node_modules/rx-lite
| LICENSE : Apache License, Version 2.0
| LICENSE_FILE : node_modules/rx-lite/readme.md
| REPOSITORY: https://github.com/Reactive-Extensions/RxJS
| PUBLISHER : Cloud Programmability Team
| EMAIL : undefined
| URL : https://github.com/Reactive-Extensions/RxJS/blob/master/authors.txt
⚠️

MODULE : node_modules/vorpal/node_modules/rx-lite
| LICENSE : Apache License, Version 2.0
| LICENSE_FILE : node_modules/vorpal/node_modules/rx-lite/readme.md
| REPOSITORY: https://github.com/Reactive-Extensions/RxJS
| PUBLISHER : Cloud Programmability Team
| EMAIL : undefined
| URL : https://github.com/Reactive-Extensions/RxJS/blob/master/authors.txt
⚠️

MODULE : node_modules/rx-lite-aggregates
| LICENSE : Apache License, Version 2.0
| LICENSE_FILE : node_modules/rx-lite-aggregates/readme.md
| REPOSITORY: https://github.com/Reactive-Extensions/RxJS
| PUBLISHER : Cloud Programmability Team
| EMAIL : undefined
| URL : https://github.com/Reactive-Extensions/RxJS/blob/master/authors.txt
⚠️

MODULE : node_modules/rc
| LICENSE : (BSD-2-Clause OR MIT OR Apache-2.0)
| LICENSE_FILE : node_modules/rc/LICENSE.APACHE2
| REPOSITORY: https://github.com/dominictarr/rc
| PUBLISHER : Dominic Tarr
| EMAIL : [email protected]
| URL : dominictarr.com
⚠️

MODULE : node_modules/qrcode-terminal
| LICENSE : Apache 2.0
| LICENSE_FILE : node_modules/qrcode-terminal/LICENSE
| REPOSITORY: https://github.com/gtanner/qrcode-terminal
| PUBLISHER : undefined
| EMAIL : undefined
| URL : undefined
⚠️

MODULE : node_modules/xhr/node_modules/process
| LICENSE : MIT*
| LICENSE_FILE : node_modules/xhr/node_modules/process/LICENSE
| REPOSITORY: https://github.com/shtylman/node-process
| PUBLISHER : Roman Shtylman
| EMAIL : [email protected]
| URL : undefined
⚠️

MODULE : node_modules/potrace
| LICENSE : GPL-2.0
| LICENSE_FILE : node_modules/potrace/LICENSE
| REPOSITORY: https://github.com/tooolbox/node-potrace
| PUBLISHER : mattmc
| EMAIL : undefined
| URL : undefined
⚠️

MODULE : node_modules/object-component
| LICENSE : MIT*
| LICENSE_FILE : node_modules/object-component/Readme.md
| REPOSITORY: undefined
| PUBLISHER : undefined
| EMAIL : undefined
| URL : undefined
⚠️

MODULE : node_modules/node-forge
| LICENSE : (BSD-3-Clause OR GPL-2.0)
| LICENSE_FILE : node_modules/node-forge/LICENSE
| REPOSITORY: https://github.com/digitalbazaar/forge
| PUBLISHER : Digital Bazaar, Inc.
| EMAIL : [email protected]
| URL : http://digitalbazaar.com/
⚠️

MODULE : node_modules/map-stream
| LICENSE : Custom: https://github.com/dominictarr/event-stream
| LICENSE_FILE : node_modules/map-stream/LICENCE
| REPOSITORY: https://github.com/dominictarr/map-stream
| PUBLISHER : Dominic Tarr
| EMAIL : [email protected]
| URL : http://dominictarr.com
⚠️

MODULE : node_modules/jsonify
| LICENSE : Public Domain
| LICENSE_FILE : node_modules/jsonify/README.markdown
| REPOSITORY: https://github.com/substack/jsonify
| PUBLISHER : Douglas Crockford
| EMAIL : undefined
| URL : http://crockford.com/
⚠️

MODULE : node_modules/json-schema
| LICENSE : AFLv2.1,BSD
| LICENSE_FILE : node_modules/json-schema/README.md
| REPOSITORY: https://github.com/kriszyp/json-schema
| PUBLISHER : Kris Zyp
| EMAIL : undefined
| URL : undefined
⚠️

MODULE : node_modules/indexof
| LICENSE : MIT*
| LICENSE_FILE : node_modules/indexof/Readme.md
| REPOSITORY: undefined
| PUBLISHER : undefined
| EMAIL : undefined
| URL : undefined
⚠️

MODULE : node_modules/glob-to-regexp
| LICENSE : BSD*
| LICENSE_FILE : node_modules/glob-to-regexp/README.md
| REPOSITORY: https://github.com/fitzgen/glob-to-regexp
| PUBLISHER : Nick Fitzgerald
| EMAIL : [email protected]
| URL : undefined
⚠️

MODULE : node_modules/font-awesome
| LICENSE : (OFL-1.1 AND MIT)
| LICENSE_FILE : node_modules/font-awesome/README.md
| REPOSITORY: https://github.com/FortAwesome/Font-Awesome
| PUBLISHER : Dave Gandy
| EMAIL : [email protected]
| URL : http://twitter.com/davegandy
⚠️

MODULE : node_modules/expand-template
| LICENSE : (MIT OR WTFPL)
| LICENSE_FILE : node_modules/expand-template/LICENSE
| REPOSITORY: https://github.com/ralphtheninja/expand-template
| PUBLISHER : LM
| EMAIL : [email protected]
| URL : undefined
⚠️

MODULE : node_modules/exif-parser
| LICENSE : MIT*
| LICENSE_FILE : node_modules/exif-parser/LICENSE.md
| REPOSITORY: https://github.com/bwindels/exif-parser
| PUBLISHER : Bruno Windels
| EMAIL : [email protected]
| URL : undefined
⚠️

MODULE : node_modules/css-select/node_modules/domutils
| LICENSE : BSD*
| LICENSE_FILE : node_modules/css-select/node_modules/domutils/LICENSE
| REPOSITORY: https://github.com/FB55/domutils
| PUBLISHER : Felix Boehm
| EMAIL : [email protected]
| URL : undefined
⚠️

MODULE : node_modules/html-dom-parser/node_modules/domhandler
| LICENSE : BSD*
| LICENSE_FILE : node_modules/html-dom-parser/node_modules/domhandler/LICENSE
| REPOSITORY: https://github.com/fb55/DomHandler
| PUBLISHER : Felix Boehm
| EMAIL : [email protected]
| URL : undefined
⚠️

MODULE : node_modules/eslint-plugin-import/node_modules/doctrine
| LICENSE : BSD
| LICENSE_FILE : node_modules/eslint-plugin-import/node_modules/doctrine/LICENSE.BSD
| REPOSITORY: https://github.com/eslint/doctrine
| PUBLISHER : undefined
| EMAIL : undefined
| URL : undefined
⚠️

MODULE : node_modules/npm/node_modules/cyclist
| LICENSE : MIT*
| LICENSE_FILE : node_modules/npm/node_modules/cyclist/README.md
| REPOSITORY: https://github.com/mafintosh/cyclist
| PUBLISHER : Mathias Buus Madsen
| EMAIL : [email protected]
| URL : undefined
⚠️

MODULE : node_modules/css-select
| LICENSE : BSD*
| LICENSE_FILE : node_modules/css-select/LICENSE
| REPOSITORY: https://github.com/fb55/css-select
| PUBLISHER : Felix Boehm
| EMAIL : [email protected]
| URL : undefined
⚠️

MODULE : node_modules/react-live/node_modules/create-react-context
| LICENSE : MIT*
| LICENSE_FILE : node_modules/react-live/node_modules/create-react-context/LICENSE
| REPOSITORY: https://github.com/thejameskyle/create-react-context
| PUBLISHER : James Kyle
| EMAIL : [email protected]
| URL : undefined
⚠️

MODULE : node_modules/component-props
| LICENSE : MIT*
| LICENSE_FILE : node_modules/component-props/Readme.md
| REPOSITORY: https://github.com/component/props
| PUBLISHER : undefined
| EMAIL : undefined
| URL : undefined
⚠️

MODULE : node_modules/component-inherit
| LICENSE : MIT*
| LICENSE_FILE : node_modules/component-inherit/Readme.md
| REPOSITORY: https://github.com/component/inherit
| PUBLISHER : undefined
| EMAIL : undefined
| URL : undefined
⚠️

MODULE : node_modules/component-bind
| LICENSE : MIT*
| LICENSE_FILE : node_modules/component-bind/Readme.md
| REPOSITORY: https://github.com/component/bind
| PUBLISHER : undefined
| EMAIL : undefined
| URL : undefined
⚠️

MODULE : node_modules/cli-table
| LICENSE : MIT*
| LICENSE_FILE : node_modules/cli-table/README.md
| REPOSITORY: https://github.com/Automattic/cli-table
| PUBLISHER : Guillermo Rauch
| EMAIL : [email protected]
| URL : undefined
⚠️

MODULE : node_modules/callsite
| LICENSE : MIT*
| LICENSE_FILE : node_modules/callsite/Readme.md
| REPOSITORY: undefined
| PUBLISHER : TJ Holowaychuk
| EMAIL : [email protected]
| URL : undefined
⚠️

MODULE : node_modules/better-assert
| LICENSE : MIT*
| LICENSE_FILE : node_modules/better-assert/Readme.md
| REPOSITORY: https://github.com/visionmedia/better-assert
| PUBLISHER : TJ Holowaychuk
| EMAIL : [email protected]
| URL : undefined
⚠️

MODULE : node_modules/async-foreach
| LICENSE : MIT*
| LICENSE_FILE : node_modules/async-foreach/LICENSE-MIT
| REPOSITORY: https://github.com/cowboy/javascript-sync-async-foreach
| PUBLISHER : "Cowboy" Ben Alman
| EMAIL : undefined
| URL : http://benalman.com/
⚠️

MODULE : node_modules/@mikaelkristiansson/domready
| LICENSE : MIT*
| LICENSE_FILE : node_modules/@mikaelkristiansson/domready/LICENSE
| REPOSITORY: https://github.com/mikaelkristiansson/domready
| PUBLISHER : Mikael Kristiansson
| EMAIL : undefined
| URL : undefined

Generated by 🚫 dangerJS against a494314

@vaporwavie vaporwavie mentioned this pull request Jul 12, 2021
Open
hmps
hmps reviewed Aug 12, 2021
View reviewed changes
react-carousel/src/components/Carousel.js
@@ -66,7 +66,9 @@ const Carousel = (props) => {
carouselRef,
);

setStrategies(strategies);
useEffect(() => {
Copy link

@hmps hmps Aug 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

👋

First things first, thank you for the work you're doing on this! 🙏

I was having the problem that this PR is aiming to fix, so I made a fork to have it available until you guys can merge and release this.

I can confirm that this change works well! 👏 We did see a new warning on the tail of it though. We had an instance of the "Can't perform a React state update on an unmounted component..." error popup on us when we moved between cards in the carousel.

On our fork, we fixed this by checking that the component is still mounted before running setStrategies. You can see our implementation at @vaam-io/react-carousel.

Keep up the good work! 🚀

EDIT: It turns out our fix didn't solve the problem at all. 🤦 We'll keep trying :)

@ssamkough
Copy link

@RobertHebel @Lukasz-pluszczewski any updates to merge this in to fix the issues? or does this PR have issues as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hmps hmps hmps left review comments

@Lukasz-pluszczewski Lukasz-pluszczewski Lukasz-pluszczewski approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@RobertHebel @ssamkough @hmps @Lukasz-pluszczewski