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

Convert to TypeScript #2063

Merged
merged 75 commits into from
Apr 12, 2021
Merged

Convert to TypeScript #2063

merged 75 commits into from
Apr 12, 2021

Conversation

aminya
Copy link
Member

@aminya aminya commented Apr 6, 2021

This converts the codebase to TypeScript

Requires:
nteract/kernelspecs#35
nteract/jupyter-paths#39

The tests

The tests are all passing
https://github.com/aminya/hydrogen/actions/runs/733861352

How to test the functionality

git clone https://github.com/aminya/hydrogen.git
cd hydrogen
git checkout typescript
apm install
npm run build
apm link .
atom .

Make sure to run apm unlink . after you're done testing, so you don't miss the official versions.

How to review

You can go through the commits which include the mental logic behind the changes.

The first commit conversion is done automatically using @khanacademy/flow-to-ts, and then the issues are fixed by hand.

If you want to verify the changes make sure to disable the whitespace changes generated by the tool.

```
npm install -g @khanacademy/flow-to-ts
flow-to-ts  --write --delete-source ./lib/**/*.js
```

In `lib/services/status-bar/status-bar-component.tsx` @observer removed before the conversion and re-added after due to prettier error
Copy link
Member Author

@aminya aminya left a comment

Choose a reason for hiding this comment

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

I am about to merge this if no one has any concerns. It would be nice if someone could test the functionality under a different workflow.

@aminya aminya requested a review from n-riesco April 10, 2021 22:10
@n-riesco
Copy link
Collaborator

@aminya I've been keeping an eye on this PR, so I could give you some comments (but not this weekend).

PS: I haven't kept up to date with nteract's libraries. I won't be able to give you feedback on their use.

@aminya
Copy link
Member Author

aminya commented Apr 10, 2021

Thanks. If you can just build and use this a bit, it would be enough.

@aminya
Copy link
Member Author

aminya commented Apr 12, 2021

I am going to merge this for now as I want to make some other changes. You can test the functionality on master.

@aminya aminya merged commit 474400f into nteract:master Apr 12, 2021
@aminya aminya deleted the typescript branch April 12, 2021 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants