Migrate codebase to Typescript #120

AjayPoshak · 2018-12-02T12:27:00Z

Summary

Moved codebase to Typescript from Flow JS. TS is fun so far 💯

Related Issue #[issue number]

Related Issue #111

Any Breaking Changes

Nope, It won't impact end user

Checklist

Are all the test cases passing?
If any new feature has been added, then are the test cases updated/added?
Has the documentation been updated for the proposed change, if required?

…odejs

ifiokjr

This looks great! 🎉

I've made some comments regarding how to make the transition seamless for current TypeScript users who have been using the DefinitelyTyped types.

tldr;

Rename IProps to ContentLoaderProps
Re-export IProps from the index.ts top level file
Explicitly define all exported components as FunctionComponent<IProps> for a better type definition file.

Hope it all makes sense 😄

ifiokjr · 2018-12-03T12:10:33Z

src/Holder.tsx

+  width: 400,
+};
+
+const InitialComponent = (props: IProps) => (


Could this be typed as

import { FunctionComponent } from 'react'; // Or -> import { FC } from 'react' to save characters const InitialComponent: FunctionComponent<IProps> = props => { // ... };

This way TS knows that this is a function component and auto injects the children and ref props for consumers.

Isn't should be React.FunctionComponent?
But great point

You're right. I'm so used to using "esModuleInterop": true, I forget it's not the default.

ifiokjr · 2018-12-03T12:12:09Z

src/Holder.tsx

+import * as React from "react";
+import Svg from "./Svg";
+
+export interface IProps {


Perhaps this can be this be renamed to ContentLoaderProps

The current type definition file has used the above name.

This would mean that current TS users won't need to change their code once the updated library is published.

Great point, but is it should start with I which means a new interface?
I prefer to follow the tslint recommendations than keep the oldest wrong version

Maybe we can update the current type definition as well

Sure thing 👍

Perhaps with a deprecation warning.

ifiokjr · 2018-12-03T12:16:21Z

src/Svg.tsx

-} & HolderProps
+import uid from "./uid";
+
+export interface ISvgProps extends IProps {


If you define a component as a FunctionComponent it doesn't need to declare children (unless you're expecting a render prop or restricting the type of children).

To enjoy this pattern you would need to name the default export.

import { FunctionComponent } from 'react'; const SomeName: FunctionComponent<IProps> = props => { // ... }; export default SomeName;

Also, children already exists in IProps, so it isn't necessary, but let's keep you suggest

ifiokjr · 2018-12-03T12:20:42Z

src/index.ts

@@ -0,0 +1,9 @@
+import Holder from "./Holder";
+


Here would be a great place to re-export the IProps or ContentLoaderProps from ./Holder.

That way users can use it in their own interfaces (all from the top level import).

danilowoz

@AjayPoshak thank you so much for your submission, it looks really great! 🥇

@ifiokjr I just leave my thought too, I hope you can contribute again :)

danilowoz · 2018-12-03T13:35:57Z

package.json

    "testonly": "cross-env NODE_ENV=test mocha $npm_package_options_mocha",
    "release:major": "npm run build && npm version major",
    "release:minor": "npm run build && npm version minor",
    "release:patch": "npm run build && npm version patch",
    "lint": "eslint 'src/**/*.js'",
-    "flow": "flow",
+    "watch": "node_modules/.bin/tsc --watch",


It would be better to say what this script is watching, like:

"tsc": "node_modules/.bin/tsc", "tsc:watch": "npm run tsc -- --watch",

danilowoz · 2018-12-03T13:44:19Z

src/Holder.tsx

+import * as React from "react";
+import Svg from "./Svg";
+
+export interface IProps {


Great point, but is it should start with I which means a new interface?
I prefer to follow the tslint recommendations than keep the oldest wrong version

Maybe we can update the current type definition as well

danilowoz · 2018-12-03T13:55:07Z

src/Holder.tsx

+import Svg from "./Svg";
+
+export interface IProps {
+  rtl?: boolean;


Actually, all props are not required;

Please sort the lines by alphabetical

danilowoz · 2018-12-03T13:55:44Z

src/Holder.tsx

+  secondaryColor?: string;
+  secondaryOpacity: number;
+  children?: React.ReactNode;
+  preserveAspectRatio?: string;


danilowoz · 2018-12-03T13:56:13Z

src/Holder.tsx

+  secondaryOpacity: number;
+  children?: React.ReactNode;
+  preserveAspectRatio?: string;
+  style: { [key: string]: any };


Please, update to this value:

style?: React.CSSProperties;

danilowoz · 2018-12-03T14:04:46Z

src/stylized/BulletListStyle.tsx


-export default BulletListStyle
+export default BulletListStyle;


Please apply these changes in the others components

danilowoz · 2018-12-03T14:07:07Z

tslint.json

+{
+  "defaultSeverity": "error",
+  "extends": [
+      "tslint:recommended"


Add the tslint-config-prettier to work fine with prettier

danilowoz · 2018-12-03T14:14:15Z

package.json

    "testonly": "cross-env NODE_ENV=test mocha $npm_package_options_mocha",
    "release:major": "npm run build && npm version major",
    "release:minor": "npm run build && npm version minor",
    "release:patch": "npm run build && npm version patch",
    "lint": "eslint 'src/**/*.js'",
-    "flow": "flow",
+    "watch": "node_modules/.bin/tsc --watch",


Missing tslint dependency and script.
Once added the tslint you can remove eslint stuff

danilowoz · 2018-12-03T14:14:46Z

package.json

-    "rollup-plugin-uglify": "^6.0.0"
+    "rollup-plugin-typescript2": "^0.18.0",
+    "rollup-plugin-uglify": "^6.0.0",
+    "ts-node": "^7.0.1",


why exactly we need this?

removed, it was needed to run mocha_test runner, but now as we have moved to jest as test runner, so we don't need it. We are using ts-jest now.

danilowoz · 2018-12-03T14:16:25Z

src/Svg.tsx

-  uniquekey,
-  width,
+  preserveAspectRatio,
+  // tslint:disable-next-line:trailing-comma


Is it really necessary? I think with the suggestion in the tslint that I did, it'll be solved

…tions

danilowoz · 2018-12-17T19:05:13Z

Really thanks @AjayPoshak for your contribution!

--

You just need to resolve the conflicts, then I can merge into develop. Don't worry if you don't know what changes it should to keep, I'll check it before I release a new version, if there is any doubt, keep your changes. About the yarn.lock you can remove the content, I can remake it later.

Again, thanks a lot 🎉
Let's move this amazing community ⚛️

AjayPoshak · 2018-12-18T04:39:47Z

@danilowoz Cool will do it.

AjayPoshak · 2018-12-22T12:43:38Z

I am migrating tests to typescript as well. This may take some time. Hopefully, I'll complete it by tomorrow.

codecov · 2018-12-23T07:29:41Z

Codecov Report

Merging #120 into develop will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           develop   #120   +/-   ##
======================================
  Coverage      100%   100%           
======================================
  Files            8      9    +1     
  Lines           23     61   +38     
  Branches         4      7    +3     
======================================
+ Hits            23     61   +38

Impacted Files	Coverage Δ
src/uid.ts	`100% <ø> (ø)`
src/Holder.tsx	`100% <100%> (ø)`
src/stylized/BulletListStyle.tsx	`100% <100%> (ø)`
src/index.ts	`100% <100%> (ø)`
src/stylized/CodeStyle.tsx	`100% <100%> (ø)`
src/Svg.tsx	`100% <100%> (ø)`
src/stylized/ListStyle.tsx	`100% <100%> (ø)`
src/stylized/FacebookStyle.tsx	`100% <100%> (ø)`
src/stylized/InstagramStyle.tsx	`100% <100%> (ø)`
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 89afd31...5a12912. Read the comment docs.

AjayPoshak · 2018-12-23T07:43:38Z

src/__tests__/uid.test.tsx

  const ids = new Array(options).fill(" ").map(item => uid())
+  // @ts-ignore To avoid adding polyfill for `from`


To use ES6 features in this file like fill, from, Set, we have to add polyfills. As this is just a test file, not affecting our library consumer in any way. So I added @ts-ignore to ignore these ts errors.

AjayPoshak · 2018-12-23T07:54:52Z

@danilowoz @ifiokjr I have made requested changes. Please review.
@danilowoz please generate yarn.lock after merging. I haven't updated lockfile.

* Ignore rpt2_cache * Add typescript pkg; Add tsc runner script * Replace babel with ts compiler * Typescript configs * Update lockfile * Convert Holder to tsx * Convert svg to tsx * Convert index.js to ts * Convert uid to ts * Convert BulletListStyle to tsx * Convert CodeStyle to tsx * Convert FacebookStyle to tsx * Convert InstagramStyle to tsx * Convert ListStyle to tsx * Remove flow preset * Add ts-node for ts execustion env for nodejs * Remove flow preset & rollup-babel plugin; Add ts-node for ts env in nodejs * Update lockfile * Update watch script for tsc * Rename IProps to IContentLoaderProps; Use FC for validation * Remove unwanted ISvgProps; import IContentLoaderProps for type validations * import and re-export IContentLoaderProps for better types * Import and use types from IContentLoaderProps * Remove eslint; replaced by tslint and tslint-prettier * Add tslint-config-prettier * Implement stricter type checking * Remove comments as it is handled by tslint-config-prettier now * Replace lint by tsc * remove eslint and flow configs * Add style default props * Add jest options to include ts test files * Add options to load json file * Migrate tests from JS to TS for better interoperability * Updated snapshots * Remove ts-node * Break line after react import * Remove export from bottom of file and move to component definition BREAKING CHANGE: Migrate codebase to typescript Closes #120

* test(Refactor): (#117) * test(Refactor): * Holder / SVG tests * Svg tests * Removed old tests * Coverage * Migrate to travis * Update travis * Update travis * ci(Release): * test(package): * ci(Travis): * chore(devDependencies): Package to generate the changelog * feat(Codebase): Format * feat(Types): Export types * ci(Build step): * feat(RTL): Flip the content BREAKING CHANGE: Flip all content instead of only flip the animation direction Closes #122 * Migrate codebase to Typescript (#120) * Ignore rpt2_cache * Add typescript pkg; Add tsc runner script * Replace babel with ts compiler * Typescript configs * Update lockfile * Convert Holder to tsx * Convert svg to tsx * Convert index.js to ts * Convert uid to ts * Convert BulletListStyle to tsx * Convert CodeStyle to tsx * Convert FacebookStyle to tsx * Convert InstagramStyle to tsx * Convert ListStyle to tsx * Remove flow preset * Add ts-node for ts execustion env for nodejs * Remove flow preset & rollup-babel plugin; Add ts-node for ts env in nodejs * Update lockfile * Update watch script for tsc * Rename IProps to IContentLoaderProps; Use FC for validation * Remove unwanted ISvgProps; import IContentLoaderProps for type validations * import and re-export IContentLoaderProps for better types * Import and use types from IContentLoaderProps * Remove eslint; replaced by tslint and tslint-prettier * Add tslint-config-prettier * Implement stricter type checking * Remove comments as it is handled by tslint-config-prettier now * Replace lint by tsc * remove eslint and flow configs * Add style default props * Add jest options to include ts test files * Add options to load json file * Migrate tests from JS to TS for better interoperability * Updated snapshots * Remove ts-node * Break line after react import * Remove export from bottom of file and move to component definition BREAKING CHANGE: Migrate codebase to typescript Closes #120

danilowoz · 2019-01-24T18:44:03Z

🎉 This issue has been resolved in version 4.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

AjayPoshak added 18 commits December 2, 2018 17:00

Ignore rpt2_cache

b7c6c7c

Add typescript pkg; Add tsc runner script

d923f31

Replace babel with ts compiler

9e42dfd

Typescript configs

3539aed

Update lockfile

52c967f

Convert Holder to tsx

e334b6c

Convert svg to tsx

311b435

Convert index.js to ts

b80eb7d

Convert uid to ts

998325a

Convert BulletListStyle to tsx

4f4bbe2

Convert CodeStyle to tsx

506e1ac

Convert FacebookStyle to tsx

fd2f3f0

Convert InstagramStyle to tsx

522e746

Convert ListStyle to tsx

3c92d5d

Remove flow preset

fef0a3a

Add ts-node for ts execustion env for nodejs

8882571

Remove flow preset & rollup-babel plugin; Add ts-node for ts env in n…

936de70

…odejs

Update lockfile

d870cb1

ifiokjr reviewed Dec 3, 2018

View reviewed changes

danilowoz requested changes Dec 3, 2018

View reviewed changes

AjayPoshak added 10 commits December 3, 2018 22:32

Update watch script for tsc

fc32447

Rename IProps to IContentLoaderProps; Use FC for validation

0b183fd

Remove unwanted ISvgProps; import IContentLoaderProps for type valida…

23a39db

…tions

import and re-export IContentLoaderProps for better types

2e1832e

Import and use types from IContentLoaderProps

7f77cd4

Remove eslint; replaced by tslint and tslint-prettier

861a54b

Add tslint-config-prettier

7ffca2c

Implement stricter type checking

b862c38

Remove comments as it is handled by tslint-config-prettier now

d19bbb9

Resolve merge conflicts

f113455

AjayPoshak added 2 commits December 7, 2018 23:48

Replace lint by tsc

f003483

remove eslint and flow configs

1ee17fa

danilowoz changed the base branch from master to develop December 17, 2018 18:52

danilowoz approved these changes Dec 17, 2018

View reviewed changes

resolve merge conflicts

a5b2dba

AjayPoshak added 5 commits December 23, 2018 12:47

Add style default props

50721fb

Add jest options to include ts test files

63c4557

Add options to load json file

e304726

Migrate tests from JS to TS for better interoperability

dcb5c56

Updated snapshots

4d5c970

Remove ts-node

d6b2c49

AjayPoshak commented Dec 23, 2018

View reviewed changes

AjayPoshak added 2 commits December 23, 2018 13:18

Break line after react import

5b1fcea

Remove export from bottom of file and move to component definition

5a12912

danilowoz merged commit 2254f25 into danilowoz:develop Jan 2, 2019

danilowoz mentioned this pull request Jan 3, 2019

Codebase migrations #126

Merged

danilowoz added the released label Jan 24, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate codebase to Typescript #120

Migrate codebase to Typescript #120

AjayPoshak commented Dec 2, 2018

ifiokjr left a comment

ifiokjr Dec 3, 2018 •

edited by danilowoz

Loading

danilowoz Dec 3, 2018

ifiokjr Dec 3, 2018

ifiokjr Dec 3, 2018

danilowoz Dec 3, 2018

ifiokjr Dec 3, 2018 •

edited

Loading

ifiokjr Dec 3, 2018 •

edited by danilowoz

Loading

danilowoz Dec 3, 2018

ifiokjr Dec 3, 2018

danilowoz Dec 3, 2018

AjayPoshak Dec 23, 2018

danilowoz left a comment •

edited

Loading

danilowoz Dec 3, 2018

danilowoz Dec 3, 2018

danilowoz Dec 3, 2018

danilowoz Dec 3, 2018

danilowoz Dec 3, 2018 •

edited

Loading

danilowoz Dec 3, 2018

danilowoz Dec 3, 2018

danilowoz Dec 3, 2018

danilowoz Dec 3, 2018

AjayPoshak Dec 23, 2018 •

edited

Loading

danilowoz Dec 3, 2018

AjayPoshak Dec 23, 2018

danilowoz commented Dec 17, 2018

AjayPoshak commented Dec 18, 2018 •

edited

Loading

AjayPoshak commented Dec 22, 2018

codecov bot commented Dec 23, 2018 •

edited

Loading

AjayPoshak Dec 23, 2018

AjayPoshak commented Dec 23, 2018 •

edited

Loading

danilowoz commented Jan 24, 2019


		export default BulletListStyle
		export default BulletListStyle;

		const ids = new Array(options).fill(" ").map(item => uid())
		// @ts-ignore To avoid adding polyfill for `from`

Migrate codebase to Typescript #120

Migrate codebase to Typescript #120

Conversation

AjayPoshak commented Dec 2, 2018

Summary

Related Issue #[issue number]

Any Breaking Changes

Checklist

ifiokjr left a comment

Choose a reason for hiding this comment

ifiokjr Dec 3, 2018 • edited by danilowoz Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ifiokjr Dec 3, 2018 • edited Loading

Choose a reason for hiding this comment

ifiokjr Dec 3, 2018 • edited by danilowoz Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danilowoz left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danilowoz Dec 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AjayPoshak Dec 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danilowoz commented Dec 17, 2018

AjayPoshak commented Dec 18, 2018 • edited Loading

AjayPoshak commented Dec 22, 2018

codecov bot commented Dec 23, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

AjayPoshak commented Dec 23, 2018 • edited Loading

danilowoz commented Jan 24, 2019

ifiokjr Dec 3, 2018 •

edited by danilowoz

Loading

ifiokjr Dec 3, 2018 •

edited

Loading

ifiokjr Dec 3, 2018 •

edited by danilowoz

Loading

danilowoz left a comment •

edited

Loading

danilowoz Dec 3, 2018 •

edited

Loading

AjayPoshak Dec 23, 2018 •

edited

Loading

AjayPoshak commented Dec 18, 2018 •

edited

Loading

codecov bot commented Dec 23, 2018 •

edited

Loading

AjayPoshak commented Dec 23, 2018 •

edited

Loading