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

Chore: Start Typescript migration #3279

Merged
merged 99 commits into from
Sep 13, 2021
Merged

Conversation

AlexAlexandre
Copy link
Contributor

@AlexAlexandre AlexAlexandre commented Jul 15, 2021

Proposed changes

The purpose of this PR is to start the project's migration to typescript.
With that, we need to pay attention to the following points:

  • Add a tsconfig.json file. In this process, I created this file with the default rules. At the moment, I'm allowing the type: any to be used, as this is the beginning of the migration, we still don't know all the types used, in the future it will be necessary to disable this option.
  • Update eslint config to support .ts and .tsx files. In this process, was necessary override some js rules, so I created separated rules to .ts. I also changed our extends rules from airbnb to @rocket.chat/eslint-config, since our company has default lint rules, that's no reason to not use it.
  • We have adopted interfaces as the default for typing in our repository, following the example below:
interface IActionSheetData {
	options: any;
	headerHeight?: number;
	hasCancel?: boolean;
	customHeader: any;
}

All interface name needs start with an I, as IActionSheetData , IAvatar, IButton... This is a common pattern, and helps all to know when see an interface on the codebase.

  • Componentes migrated:
    • app/constants;
    • app/containers;
    • app/presentation;
    • In this process, was necessary migrated some separate files, for example: app/lib/Navigation.ts and app/lib/ShareNavigation.ts. To use your resources elsewhere.

Prettier

Taking advantage of all the changes made on lint, I also set up prettier in the project, to help us with te code formatting.
This tool has very interesting features, and I recommend that all developers involved in this project configure their IDE for automatic code checking and integration.

To more information, please see the link: Prettier.

Thinking about ensuring the quality of the code, I took the liberty of changing our pre-commit script, to not only run lint, but also check the formatting of the code using prettier. The new script now runs this command:

"prettier-lint": "yarn lint --fix && prettier --write ."

Important

Depends on RocketChat/Rocket.Chat.js.SDK#145
Depends on RocketChat/react-native-ui-lib#3

Issue(s)

Closes #786

How to test or reproduce

We can test the prettier following this steps:
To check your code formatting, run this on your terminal:

yarn prettier-check

To fix your code formatting issues, run this on your terminal:

yarn prettier

Screenshots

N/A

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

@diegolmello diegolmello changed the title Improvement.typescript migration Chore: Start Typescript migration Jul 15, 2021
@lgtm-com
Copy link

lgtm-com bot commented Jul 15, 2021

This pull request introduces 1 alert when merging 802c7b2 into d1f35bb - view on LGTM.com

new alerts:

  • 1 for Use of returnless function

@lgtm-com
Copy link

lgtm-com bot commented Jul 15, 2021

This pull request introduces 1 alert when merging acdc2ce into d1f35bb - view on LGTM.com

new alerts:

  • 1 for Use of returnless function

@lgtm-com
Copy link

lgtm-com bot commented Jul 15, 2021

This pull request introduces 1 alert and fixes 1 when merging a18b13c into d1f35bb - view on LGTM.com

new alerts:

  • 1 for Direct state mutation

fixed alerts:

  • 1 for Direct state mutation

@lgtm-com
Copy link

lgtm-com bot commented Jul 16, 2021

This pull request introduces 1 alert and fixes 1 when merging 19d8ab1 into d1f35bb - view on LGTM.com

new alerts:

  • 1 for Direct state mutation

fixed alerts:

  • 1 for Direct state mutation

@lgtm-com
Copy link

lgtm-com bot commented Jul 19, 2021

This pull request introduces 1 alert and fixes 1 when merging f5eee1c into d1f35bb - view on LGTM.com

new alerts:

  • 1 for Direct state mutation

fixed alerts:

  • 1 for Direct state mutation

@lgtm-com
Copy link

lgtm-com bot commented Jul 19, 2021

This pull request introduces 1 alert and fixes 1 when merging d060edb into d1f35bb - view on LGTM.com

new alerts:

  • 1 for Direct state mutation

fixed alerts:

  • 1 for Direct state mutation

@lgtm-com
Copy link

lgtm-com bot commented Sep 8, 2021

This pull request introduces 1 alert and fixes 1 when merging 5ee7cda into 9c526b7 - view on LGTM.com

new alerts:

  • 1 for Direct state mutation

fixed alerts:

  • 1 for Direct state mutation

@lgtm-com
Copy link

lgtm-com bot commented Sep 8, 2021

This pull request introduces 1 alert and fixes 1 when merging 507bf76 into 9c526b7 - view on LGTM.com

new alerts:

  • 1 for Direct state mutation

fixed alerts:

  • 1 for Direct state mutation

@lgtm-com
Copy link

lgtm-com bot commented Sep 9, 2021

This pull request introduces 1 alert and fixes 1 when merging 5e5a886 into 9c526b7 - view on LGTM.com

new alerts:

  • 1 for Direct state mutation

fixed alerts:

  • 1 for Direct state mutation

@lgtm-com
Copy link

lgtm-com bot commented Sep 9, 2021

This pull request introduces 1 alert and fixes 1 when merging d824d86 into 9c526b7 - view on LGTM.com

new alerts:

  • 1 for Direct state mutation

fixed alerts:

  • 1 for Direct state mutation

@lgtm-com
Copy link

lgtm-com bot commented Sep 10, 2021

This pull request introduces 1 alert and fixes 1 when merging be4b3e8 into 9c526b7 - view on LGTM.com

new alerts:

  • 1 for Direct state mutation

fixed alerts:

  • 1 for Direct state mutation

Copy link
Member

@diegolmello diegolmello left a comment

Choose a reason for hiding this comment

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

LGTM :)

@lgtm-com
Copy link

lgtm-com bot commented Sep 10, 2021

This pull request introduces 1 alert and fixes 1 when merging cb6eefa into 9c526b7 - view on LGTM.com

new alerts:

  • 1 for Direct state mutation

fixed alerts:

  • 1 for Direct state mutation

@diegolmello diegolmello force-pushed the improvement.typescript-migration branch from f38633d to ada8583 Compare September 13, 2021 16:32
@lgtm-com
Copy link

lgtm-com bot commented Sep 13, 2021

This pull request introduces 1 alert and fixes 1 when merging ada8583 into 9c526b7 - view on LGTM.com

new alerts:

  • 1 for Direct state mutation

fixed alerts:

  • 1 for Direct state mutation

@lgtm-com
Copy link

lgtm-com bot commented Sep 13, 2021

This pull request introduces 1 alert and fixes 1 when merging 8d56490 into 9c526b7 - view on LGTM.com

new alerts:

  • 1 for Direct state mutation

fixed alerts:

  • 1 for Direct state mutation

@diegolmello diegolmello merged commit 69a67ea into develop Sep 13, 2021
@diegolmello diegolmello deleted the improvement.typescript-migration branch September 13, 2021 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Typescript
2 participants