Skip to content

TS migration of 'transformer' and 'layouter' #473

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
Merged
merged 38 commits into from
Jul 18, 2019
Merged

TS migration of 'transformer' and 'layouter' #473

merged 38 commits into from
Jul 18, 2019

Conversation

yeze322
Copy link
Contributor

@yeze322 yeze322 commented Jul 10, 2019 •
edited
Loading

Description

We've migrated to TS and TSX in recent changes, however, tslint & type system are not actually working due to so many missing types.

This PR focuses on making transformers and layouters typed. Some not well defined interfaces are also refactored based on TS type system.

Touched files:
image

Why they were changed?

  • components:
    • follow transformers' return value type changes
    • reorganize import statement due to rename
  • editors: ditto
  • layouters:
    • add missing argument types
    • add missing return value types
    • (calculateNodeBoundary.ts) use Boudary as input params
  • shared
    • add some missing types
    • create new schema
    • move in/out some files
  • transformers:
    • add missing input param types
    • return NULL when input invalid data (this changed the invocation behavior under components)
    • remove the unhappy StepGroup.children transformation in all transformers (it's moved into component StepGroup)
  • index.tsx: fix unsafe visit to a possibly undefined object

Task Item

No related task.

Type of change

  • Code refactor (non-breaking change which improve code quality, clean up, add tests, etc)

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have functionally tested my change

Screenshots

No UX changes

@yeze322 yeze322 changed the title Some refactoring works based on ts type system Refactoring on 'transformer' and 'layouter' based on ts type system Jul 10, 2019
a-b-r-o-w-n
a-b-r-o-w-n requested changes Jul 10, 2019
View reviewed changes
@yeze322 yeze322 changed the title Refactoring on 'transformer' and 'layouter' based on ts type system TS types for 'transformer' and 'layouter' Jul 15, 2019
@alanlong9278 @yeze322
Julong/layouter test (#545)
4e57ed5 
* test

* calculate function

* layouter test

* measureJson

* test

* calculate function

* layouter test

* measureJson
@yeze322
Copy link
Contributor Author

yeze322 commented Jul 15, 2019

Alan added some more test cases for layouters. @a-b-r-o-w-n @boydc2014 any other suggestions?

cwhitten
cwhitten previously requested changes Jul 15, 2019
View reviewed changes
Copy link
Member

@cwhitten cwhitten left a comment

Choose a reason for hiding this comment

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

Let's take another pass at the any type definitions to see if we can provide some better shape

@yeze322
Copy link
Contributor Author

yeze322 commented Jul 16, 2019

@cwhitten Sorry for those confusing 'any' types. I've pushed a lot of changes to make types in 'transformers' and 'layouters' more explicit.

@alanlong9278 alanlong9278 mentioned this pull request Jul 16, 2019
Merged
9 tasks
@yeze322 yeze322 changed the title TS types for 'transformer' and 'layouter' TS migration of 'transformer' and 'layouter' Jul 18, 2019
@a-b-r-o-w-n a-b-r-o-w-n dismissed cwhitten’s stale review July 18, 2019 18:36

reviewed by me!

@a-b-r-o-w-n a-b-r-o-w-n merged commit db14e48 into master Jul 18, 2019
@a-b-r-o-w-n a-b-r-o-w-n deleted the zeye/ts-mig branch July 18, 2019 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@a-b-r-o-w-n a-b-r-o-w-n a-b-r-o-w-n approved these changes

@luhan2017 luhan2017 Awaiting requested review from luhan2017

@cwhitten cwhitten Awaiting requested review from cwhitten cwhitten is a code owner

@boydc2014 boydc2014 Awaiting requested review from boydc2014 boydc2014 is a code owner

Assignees
No one assigned
Labels
Area: Visual editor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@yeze322 @cwhitten @a-b-r-o-w-n @boydc2014 @alanlong9278