[TS migration] Setup typescript in expensify-common & Migrate CONST, fastMerge, Device, Logger to Typescript#699
Conversation
|
@tgolen I thought I'd bring you in to review this one, since I may not be the best suited to confirm that everything is as it should... I haven't seen anything wrong in the code, but I think a second more veteran opinion is worth it here. |
|
I'm really sorry. I was not able to review this today due to being heads-down getting comp review ready. I will be OOO for the next 1.5 week, so I will assign someone else to review. |
roryabraham
left a comment
There was a problem hiding this comment.
Also, what is the plan for publishing this? Right now, expensify-common is consumed via a git hash in E/App's package.json
That could work if we commit dist, but I see you've addd it to the .gitignore. So what's the plan? If we need to publish this to npm can you copy the workflow from react-native-onyx?
I think copying the workflow from |
|
@roryabraham I copied over the
FYI: There is If you think this PR is too big, I can move publish workflow and related changes to a separate PR. |
|
Also, please have another look at the code, I changed it slightly! 🚀 |
roryabraham
left a comment
There was a problem hiding this comment.
LGTM but let's get those secrets added before we merge
|
Created internal ticket to add those GitHub secrets |
|
Let's see if publishing works as expected... https://github.com/Expensify/expensify-common/actions/runs/9261388961 |
|
🚀Published to npm in v2.0.1 |
|
Looks like that worked (after a few more minor adjustments) 🎉 |
|
Awesome! Thanks @roryabraham for the help on this one 🙌 |
Going forward expensify-common code can be written in Typescript, we have to take into consideration transpiling step now.
TS code will live in
libcatalog and transpiled code will end up indistcatalog. Thelibcatalog won’t be published to npm; instead, we’ll only publish thedistcatalog.expensify-commonimports will look like this:Here is a similar PR that prepared Onyx for a similar migration.
Fixed Issues
$ Expensify/App#42245