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(v6): TypeScript proto #7839

Closed
wants to merge 55 commits into from
Closed

chore(v6): TypeScript proto #7839

wants to merge 55 commits into from

Conversation

ShaMan123
Copy link
Contributor

@ShaMan123 ShaMan123 commented Mar 29, 2022

Introduce a transformer that is able to convert most of the files under src into es6/ts and produces a good diff on git when preserving the file names (changing the extension is OK by git).

Relevant changes

  • scripts/transform_file.js - the transformer logic
  • scripts/index.js - expose cmd for cli (npm run dev transform -- --help)
  • src files were committed to show the diff - they shouldn't be accepted into master yet
  • build.js - 3e8cf5f reverted the following:
    The build file can do the following:
    1. compile ts/js files from src to dist
    2. build fabric.js from compiled files.
  • src/util/anim_ease.ts is a manual converted file

build needs to be replaced and babel introduced

ShaMan123 and others added 8 commits March 29, 2022 14:45
@ShaMan123 ShaMan123 requested review from melchiar and asturur March 29, 2022 12:42
@ShaMan123 ShaMan123 marked this pull request as ready for review March 29, 2022 13:09
@github-actions
Copy link
Contributor

github-actions bot commented Mar 29, 2022

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
----------|---------|----------|---------|---------|-------------------
All files |       0 |        0 |       0 |       0 |                   
----------|---------|----------|---------|---------|-------------------

@github-actions
Copy link
Contributor

github-actions bot commented Mar 29, 2022

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
----------|---------|----------|---------|---------|-------------------
All files |       0 |        0 |       0 |       0 |                   
----------|---------|----------|---------|---------|-------------------

@ShaMan123 ShaMan123 marked this pull request as draft March 29, 2022 13:14
@github-actions
Copy link
Contributor

github-actions bot commented Mar 29, 2022

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
----------|---------|----------|---------|---------|-------------------
All files |       0 |        0 |       0 |       0 |                   
----------|---------|----------|---------|---------|-------------------

package.json Outdated Show resolved Hide resolved
@asturur
Copy link
Member

asturur commented Apr 1, 2022

@ShaMan123 as soon as the group pr is merged, we can start this.
My plan is:
first pr:

  • convert 1 file that has only functions ( easing file is a good example )
  • get a new eslint config that support ts syntax and tslint config
  • choose the basic transpile target ( cant' be es5 )
  • choose the new bundler / packager ( because the old build script won't work since wrapping function won't be there and so it will pollute the global space, or modify the current one to work.
  • no classes, no mixins nothing

then we rotate a class per pr, possibly let's involve whoever wants to convert, so that everyone can get dirty hands and get comfortable with the process. I want to do at least one.

@ShaMan123
Copy link
Contributor Author

sounds good to me

@ShaMan123
Copy link
Contributor Author

should use tsc --watch option as it is incremental

@ShaMan123 ShaMan123 changed the title chore(v6):TypeScript proto chore(v6): TypeScript proto Apr 21, 2022
@ShaMan123 ShaMan123 added the v6 label Apr 23, 2022
commit b22237c
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sun Jun 12 00:12:17 2022 +0300

    Update transform_file.js

commit 79beda5
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sun Jun 12 00:10:39 2022 +0300

    Update transform_file.js

commit afb477d
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sun Jun 12 00:09:09 2022 +0300

    Update transform_file.js

commit 2c44f80
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sun Jun 12 00:04:05 2022 +0300

    Update index.js

commit 42de37a
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sun Jun 12 00:02:28 2022 +0300

    Update index.js

commit 1c7cd0a
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 23:59:06 2022 +0300

    Update transform_file.js

commit c6bb365
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 23:48:52 2022 +0300

    Update index.js

commit a978def
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 23:47:06 2022 +0300

    expose on cli

commit 25f5282
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 23:29:41 2022 +0300

    Update transform_file.js

commit 69f5810
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 23:21:34 2022 +0300

    Update transform_file.js

commit a1af13c
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 23:08:22 2022 +0300

    Update transform_file.js

commit d68a89e
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 23:07:10 2022 +0300

    Update transform_file.js

commit 98e3e12
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 23:06:26 2022 +0300

    fix "fix NS var usage"

    This fixes  a regression made by commit af1aa03.

commit af1aa03
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 22:31:37 2022 +0300

    fix NS var usage

commit a32ee7b
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 22:00:27 2022 +0300

    Update transform_file.js

commit 7475b75
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 21:57:54 2022 +0300

    Update transform_file.js

commit dc90fe8
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 21:57:36 2022 +0300

    Update transform_file.js

commit 8cd097c
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 21:43:24 2022 +0300

    Update transform_file.js

commit ca4a619
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 21:17:49 2022 +0300

    build passes

commit a8ed02a
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 21:16:23 2022 +0300

    Update transform_file.js

commit d408f4d
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 19:45:55 2022 +0300

    fix comments bug

commit 1c95448
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 19:23:50 2022 +0300

    Update transform_file.js

commit f69f37b
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 19:14:17 2022 +0300

    Update transform_file.js

commit 48aefbe
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 19:12:04 2022 +0300

    Update transform_file.js

commit 0416ea2
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 19:02:19 2022 +0300

    Update transform_file.js

commit ec4b362
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 18:59:31 2022 +0300

    Update transform_file.js

commit 5a4fc01
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 18:51:30 2022 +0300

    revert

commit 0f36da9
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 18:48:40 2022 +0300

    Update transform_file.js

commit 4bdbb5d
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 18:45:18 2022 +0300

    Update transform_file.js

commit a4adba8
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 18:44:36 2022 +0300

    Update transform_file.js

commit 41a1dff
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 18:43:12 2022 +0300

    Update transform_file.js

commit 8257bc6
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 18:41:47 2022 +0300

    Update transform_file.js

commit 953a020
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 18:34:06 2022 +0300

    Update transform_file.js

commit cff5157
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 18:31:58 2022 +0300

    Update transform_file.js

commit d0b1835
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sat Jun 11 18:27:53 2022 +0300

    revert ts change
Copy link
Contributor Author

@ShaMan123 ShaMan123 left a comment

Choose a reason for hiding this comment

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

@asturur @melchiar take a look at 04be3f5 (it is only a POC - after we merge all the PRs we should rerun the transformer)! I managed to build a transformer that works pretty good. I'm really happy about it (accept for the horrible time sink)!
And I saw something very exciting in VSCODE:

group.class.ts.-.fabric.js.-.Visual.Studio.Code.2022-06-12.00-25-29.mp4

Then human work can be much easier:

  • static properties - too hard to impl in transformer
  • imports/export/build/
  • sub/super/abstract class
  • cleanup
  • types
  • private/protected - can be done with the transformer
  • things missed out by the transformer (see dump.txt)

OHHH
Forgot this:
Run npm run dev transform to run the transformer (it will fail for transformed files - revert the transformation commit 04be3f5 before running the cmd)

@ShaMan123
Copy link
Contributor Author

ShaMan123 commented Jun 12, 2022

Transforming PRs is now possible!
I have tested the following:

  • mock merging a PR into master (merged deprecate-extend into test-pr-transform)
  • ran npm run dev transform -- -ts -o --diff
  • committed changes 0b6da98
  • merged branch into mocked transformed master 656bc9d (merged test-pr-transform into ts-proto)
  • resolved conflicts

The conflicts were simple and weren't affected by the conversion.

Copy link
Contributor Author

@ShaMan123 ShaMan123 left a comment

Choose a reason for hiding this comment

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

The transformer is in a good state
We can revert the changes to src and dist and merge this PR as it only changes scripts

@ShaMan123
Copy link
Contributor Author

updated description

@ShaMan123
Copy link
Contributor Author

closing in favor of #8020

@ShaMan123 ShaMan123 closed this Jun 23, 2022
@ShaMan123 ShaMan123 deleted the ts-proto branch September 12, 2022 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants