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

Migrate last 2 packages to TS (10/n) #2237

Merged
merged 8 commits into from
May 24, 2022
Merged

Conversation

thegreatercurve
Copy link
Contributor

@thegreatercurve thegreatercurve commented May 22, 2022

Closes #1242! This PR includes:

  • Migrated 2 packages:
    • lexical
    • lexical-yjs
  • Remove all Flow build configuration
  • Remove and rename shared-ts folder to just shared.

I've had to add some any types in the Lexical YJS packages as the types are slightly unclear.

Migration steps

I ran the below script to update the directories automatically, and then fixed each compiler linter error manually:

#!/usr/bin/env bash
DIRECTORIES=(
    "lexical"
    "lexical-yjs"
)

for directory in ${DIRECTORIES[@]}; do
    # Preserve commit history
    for i in $(find ./packages/$directory -iname "*.js" -not -path "*dist*");
        do git mv "$i" "$(echo $i | rev | cut -d '.' -f 2- | rev).ts";
    done
    for i in $(find ./packages/$directory -iname "*.jsx" -not -path "*dist*");
        do git mv "$i" "$(echo $i | rev | cut -d '.' -f 2- | rev).tsx";
    done

    for i in $(find ./packages/$directory -iname "*.ts" -not -path "*dist*" -not -path "flow");
        do sed -i "" "s/boolean %checks/node is ManuallyFindAndReplace/g" $i && sed -i "" "s/: LexicalNode>/ = LexicalNode>/g" $i;
    done
    for i in $(find ./packages/$directory -iname "*.tsx" -not -path "*dist*" -not -path "flow");
        do sed -i "" "s/boolean %checks/node is ManuallyFindAndReplace/g" $i && sed -i "" "s/: LexicalNode>/ = LexicalNode>/g" $i;
    done
    
    npx @khanacademy/flow-to-ts --write "./packages/$directory/**/{!*.d.ts,*.ts}";
    npx @khanacademy/flow-to-ts --write "./packages/$directory/**/*.tsx";
done

npm run prettier:fix;

Bundle sizes

Bundle sizes are consistent with main. Below are some sample packages:

package main TS branch
Lexical.prod.js 75K 75K
LexicalYJS.prod.js 17K 17K

@vercel
Copy link

vercel bot commented May 22, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
lexical ✅ Ready (Inspect) Visit Preview May 24, 2022 at 8:21AM (UTC)
lexical-playground ✅ Ready (Inspect) Visit Preview May 24, 2022 at 8:21AM (UTC)

@yuzhouu
Copy link

yuzhouu commented May 23, 2022

As all code has been migrated to ts, shouldn't we remove *.d.ts file?

@thegreatercurve
Copy link
Contributor Author

As all code has been migrated to ts, shouldn't we remove *.d.ts file?

@yuzhouu We should, but I need to make sure that the auto-generated complimentary Flow Types work with our internal codebase.

@zurfyx
Copy link
Member

zurfyx commented May 23, 2022

As all code has been migrated to ts, shouldn't we remove *.d.ts file?

@yuzhouu We should, but I need to make sure that the auto-generated complimentary Flow Types work with our internal codebase.

The .d.ts definition also allows us to refine types and visibility (like hiding private properties), no?

Copy link
Member

@zurfyx zurfyx left a comment

Choose a reason for hiding this comment

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

Great work! 😍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate the codebase from flow to typescript
4 participants