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

Pass State Types to Swiper Component #1076

Merged
merged 2 commits into from
Oct 3, 2019
Merged

Conversation

liamfd
Copy link
Contributor

@liamfd liamfd commented Oct 2, 2019

Is it a bugfix ?

  • Yes or No ? No
  • If yes, which issue (fix #number) ? I didn't see an open issue for this

Is it a new feature ?

  • Yes or no ? No (I don't think so)
  • Include documentation, demo GIF if applicable

Describe what you've done:

In trying to do something along the lines of:

  const swiper = React.useRef<ReactNativeSwiper>(null);
  
  React.useEffect(() => {
      if (!swiper.current) {
        return;
      }
      if (swiper.current.state.index !== indexOfNextQuestion) {
        swiper.current.scrollBy(
          indexOfNextQuestion - swiper.current.state.index,
          true
        );
  }

TypeScript was saying "Property 'index' does not exist on type 'Readonly<{}>'." on swiper.current.state.index, even though the value was defined at runtime and these values appeared to be defined in state in the component's source code.

In looking at the types, it looked like the state wasn't being passed to the React.Component type, so it made sense that TS believed that the state object was empty. Applying this change resolves that.

How to test it ?

I've tested it in my project. I imagine anything that tries to pull state off of a ref will encounter this issue. Happy to try to put together a quick test project if that would be useful.

Thank you for your work on this project, please let me know if I there's anything I can do to improve this PR!

@ArrayZoneYour
Copy link
Collaborator

@liamfd Good job👍
Can you rename SwiperStates to SwiperState by the way? I think xxxState is better.

@ArrayZoneYour ArrayZoneYour self-requested a review October 3, 2019 06:53
@liamfd
Copy link
Contributor Author

liamfd commented Oct 3, 2019

@ArrayZoneYour thanks! Renamed it to SwiperState, I like that naming too.

@ArrayZoneYour ArrayZoneYour merged commit 9e50938 into leecade:master Oct 3, 2019
pandamako added a commit to Complead/react-native-swiper that referenced this pull request Dec 24, 2019
* upstream/master: (31 commits)
  Pass State Types to Swiper Component (leecade#1076)
  Add missing scrollTo TypeScript definition (leecade#1036)
  feat(vertical): partical support vertical scrolling (leecade#1017)
  fix(children): fix the crash when only one children (leecade#1016)
  chore(package): update standard to version 13.0.1 (leecade#1014)
  fix(types): allow the style to be array (leecade#1013)
  docs(nightly): add nightly version updates (leecade#1012)
  feat(pages): support optionally render page (leecade#1009)
  Bump handlebars from 4.0.10 to 4.1.2 in /examples
  Bump stringstream from 0.0.5 to 0.0.6 in /examples (leecade#994)
  add array to dotStyles propTypes (leecade#598)
  Bump diff from 3.3.0 to 3.5.0 in /examples (leecade#990)
  chore(package): update snazzy to version 7.1.0 (leecade#736)
  chore(package): update standard to version 11.0.0 (leecade#733)
  Added prop to disable the PrevButton (leecade#749)
  fix(autoplay): replay when autoplay is setted to true
  Remove Arial font style
  fix(types): correct the wrong types
  docs(readme): remove ad
  docs(v1.6.0-dev): update changelog and installation guide
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants