Skip to content

Conversation

@jgonet
Copy link
Member

@jgonet jgonet commented Dec 28, 2020

Description

Session 3: Honky Tonk Rewrite

Last part of the TS rewrite process, in this PR we transfer types from declaration files to their implementations and add missing types for other functions and components.

Change lists are probably not exhaustive but we tried to write most of the changes.

Changes

TS

  • Removed declaration files for Swipeable and DrawerLayout components.
  • Added types compatibility layer for the new types (gestureHandlerTypesCompat.ts).
  • Added generation of declarations and declarations map when TS is transpiled.

Internal

  • Added types for all files used in RN.
  • Added basic types for the web version.
  • Mark web base classes as abstract (DiscreteGH, DraggingGH, etc).

Source code

  • Added "browser" target and updated targets for source and types for Swipeable and DrawerLayout components package.jsons.
  • Removed babel config for the main module, moved to examples.
  • Changed "react-native" entry point to index.ts (this ensures that clients using RN will load source instead of TS transpiled files).
  • Changed "types" to "dist/index.d.ts".

Internal

  • Changed most underscore prefixed function to non-prefixed, private functions.
  • Removed not used functions and arguments.
  • Replaced x && x() with x.?() in various places.
  • Removed prop types and replace them with lists of strings when necessary.

Misc

Internal

  • Changed some TS-specific Eslint rules to "warn" mode.
  • Removed no-use-before-define rule.
  • Example's Eslint config extends root config.
  • Added support for the web version of Examples.
  • State and NativeViewGestureHandler are now named export internally.
  • Renamed Gestures to gestureHandlers .
  • Added lint npm script.

TODO

  • Fill diff between master / ts-rewrite and this PR
  • Update links referencing files in the docs
  • List all deprecations and breaking changes
  • List all notable changes
  • Review TODOs, eslint-ignores, @ts-ignores, and @ts-nochecks

Missing types

  • src/components/GestureComponents.web.tsx
  • src/web/DiscreteGestureHandler.ts
  • src/web/DraggingGestureHandler.ts
  • src/web/FlingGestureHandler.ts
  • src/web/GestureHandler.ts
  • src/web/IndiscreteGestureHandler.ts
  • src/web/LongPressGestureHandler.ts
  • src/web/NativeViewGestureHandler.ts
  • src/web/PanGestureHandler.ts
  • src/web/PinchGestureHandler.ts
  • src/web/PressGestureHandler.ts
  • src/web/RotationGestureHandler.ts
  • src/web/TapGestureHandler.ts

Further improvements mentioned in review comments

Test plan

This will be tested by manually checking examples.

Future PRs can be checked by comparing bundle output (create two packages from master and this branch using npm pack, install one and then second later, create two bundles via yarn react-native bundle --entry-file=index.js --bundle-output=rngh.bundle --reset-cache --minify=false, and compare results with diff rngh.bundle rngh.ts.bundle). This PR is too big (18k diff) so we didn't do that.

export const DrawerLayoutAndroid = createNativeWrapper(RNDrawerLayoutAndroid, {
disallowInterruption: true,
});
// @ts-ignore -- TODO(TS) to investigate if it's needed
Copy link
Member

Choose a reason for hiding this comment

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

We talked about it sometime ago and I think we should at least deprecate usage of this to encourage people to use literal values of "left" and "right".

Copy link
Member

Choose a reason for hiding this comment

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

Oh, it is web, so I am not sure.

Copy link
Member

@WoLewicki WoLewicki left a comment

Choose a reason for hiding this comment

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

Left some comments, looks good overall!

export const DrawerLayoutAndroid = createNativeWrapper(RNDrawerLayoutAndroid, {
disallowInterruption: true,
});
// @ts-ignore -- TODO(TS) to investigate if it's needed
Copy link
Member

Choose a reason for hiding this comment

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

Oh, it is web, so I am not sure.

@jgonet jgonet marked this pull request as ready for review February 2, 2021 16:34
@jgonet jgonet merged commit a3a6c0a into ts-rewrite Feb 2, 2021
@jgonet jgonet deleted the @wolewicki/configure-ts branch February 2, 2021 17:42
@jgonet jgonet mentioned this pull request Feb 2, 2021
jgonet added a commit that referenced this pull request Feb 8, 2021
## Description

This PR combines three others (#1273, #1280, #1291) and rewrites Gesture Handler and examples to TS.
We tried to make non-breaking changes but this should be tested.

Most types were overhauled, although they should remain compatible with old ones thanks to the type aliases in the `src/handlers/gestureHandlerTypesCompat.ts` file.

CI for iOS was disabled due to strange fails. It should be reenabled when we add unit/e2e tests to RNGH.

Fixes #1076 - removed Flow from DrawerLayout
Fixes #1261 - added TS types
Fixes #998 - regenerated example app
Fixes #887 - removed prop types

## Changes

### Package packing, configs & source code

- Swipeable and DrawerLayout are now resolved by their own `package.json`s
- React Native uses source code directly, browser and TS use files from `dist/`
- Removed prop types

#### Internal

- Removed unused babel config in the root dir, adjust Eslint config
- Removed unused functions
- Changed private names from `_name` to `private name`
- Replaced `x && x()` syntax with `x.?()`

### Types

- Added types compatibility layer for the new types (`gestureHandlerTypesCompat.ts`)
- Added generation of declarations and declarations map when TS is transpiled
- Moved types from declaration files to files with implementation
- Added types to Android / iOS / shared JS code
- Added basic types to the web version

### Example app

- Removed unused bits in examples
- Regenerated Example app with latest RN
- Removed stray files from examples/
- Updated Travis config to include newer Android build tools & iOS scheme name change
- Organized Example code structure
- Removed Example tests (they weren't testing anything)
- Reconfigured web version of examples

## Testing

PRs that were merged into `ts-rewrite` branch were tested by using `npm pack` and adding it to the new app. In the case of #1273 & #1280, we additionally checked bundled diff for changes.

We diffed master to generated dist/ directory using `diff --color -wryN -W 200 src/ dist/src`with imports removed by `find src -name "*.js" -type f -exec sed -i '' '/import/d' {} \;`

Co-authored-by: WoLewicki <[email protected]>
Co-authored-by: Jakub <[email protected]>
mrousavy pushed a commit to mrousavy/react-native-gesture-handler that referenced this pull request Feb 11, 2021
This PR combines three others (software-mansion#1273, software-mansion#1280, software-mansion#1291) and rewrites Gesture Handler and examples to TS.
We tried to make non-breaking changes but this should be tested.

Most types were overhauled, although they should remain compatible with old ones thanks to the type aliases in the `src/handlers/gestureHandlerTypesCompat.ts` file.

CI for iOS was disabled due to strange fails. It should be reenabled when we add unit/e2e tests to RNGH.

Fixes software-mansion#1076 - removed Flow from DrawerLayout
Fixes software-mansion#1261 - added TS types
Fixes software-mansion#998 - regenerated example app
Fixes software-mansion#887 - removed prop types

- Swipeable and DrawerLayout are now resolved by their own `package.json`s
- React Native uses source code directly, browser and TS use files from `dist/`
- Removed prop types

- Removed unused babel config in the root dir, adjust Eslint config
- Removed unused functions
- Changed private names from `_name` to `private name`
- Replaced `x && x()` syntax with `x.?()`

- Added types compatibility layer for the new types (`gestureHandlerTypesCompat.ts`)
- Added generation of declarations and declarations map when TS is transpiled
- Moved types from declaration files to files with implementation
- Added types to Android / iOS / shared JS code
- Added basic types to the web version

- Removed unused bits in examples
- Regenerated Example app with latest RN
- Removed stray files from examples/
- Updated Travis config to include newer Android build tools & iOS scheme name change
- Organized Example code structure
- Removed Example tests (they weren't testing anything)
- Reconfigured web version of examples

PRs that were merged into `ts-rewrite` branch were tested by using `npm pack` and adding it to the new app. In the case of software-mansion#1273 & software-mansion#1280, we additionally checked bundled diff for changes.

We diffed master to generated dist/ directory using `diff --color -wryN -W 200 src/ dist/src`with imports removed by `find src -name "*.js" -type f -exec sed -i '' '/import/d' {} \;`

Co-authored-by: WoLewicki <[email protected]>
Co-authored-by: Jakub <[email protected]>
braincore pushed a commit to braincore/react-native-gesture-handler that referenced this pull request Mar 4, 2021
## Description

This PR combines three others (software-mansion#1273, software-mansion#1280, software-mansion#1291) and rewrites Gesture Handler and examples to TS.
We tried to make non-breaking changes but this should be tested.

Most types were overhauled, although they should remain compatible with old ones thanks to the type aliases in the `src/handlers/gestureHandlerTypesCompat.ts` file.

CI for iOS was disabled due to strange fails. It should be reenabled when we add unit/e2e tests to RNGH.

Fixes software-mansion#1076 - removed Flow from DrawerLayout
Fixes software-mansion#1261 - added TS types
Fixes software-mansion#998 - regenerated example app
Fixes software-mansion#887 - removed prop types

## Changes

### Package packing, configs & source code

- Swipeable and DrawerLayout are now resolved by their own `package.json`s
- React Native uses source code directly, browser and TS use files from `dist/`
- Removed prop types

#### Internal

- Removed unused babel config in the root dir, adjust Eslint config
- Removed unused functions
- Changed private names from `_name` to `private name`
- Replaced `x && x()` syntax with `x.?()`

### Types

- Added types compatibility layer for the new types (`gestureHandlerTypesCompat.ts`)
- Added generation of declarations and declarations map when TS is transpiled
- Moved types from declaration files to files with implementation
- Added types to Android / iOS / shared JS code
- Added basic types to the web version

### Example app

- Removed unused bits in examples
- Regenerated Example app with latest RN
- Removed stray files from examples/
- Updated Travis config to include newer Android build tools & iOS scheme name change
- Organized Example code structure
- Removed Example tests (they weren't testing anything)
- Reconfigured web version of examples

## Testing

PRs that were merged into `ts-rewrite` branch were tested by using `npm pack` and adding it to the new app. In the case of software-mansion#1273 & software-mansion#1280, we additionally checked bundled diff for changes.

We diffed master to generated dist/ directory using `diff --color -wryN -W 200 src/ dist/src`with imports removed by `find src -name "*.js" -type f -exec sed -i '' '/import/d' {} \;`

Co-authored-by: WoLewicki <[email protected]>
Co-authored-by: Jakub <[email protected]>
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.

6 participants