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

Epic: TypeScript rollout #6392

Closed
pettinarip opened this issue May 18, 2022 · 49 comments
Closed

Epic: TypeScript rollout #6392

pettinarip opened this issue May 18, 2022 · 49 comments
Assignees
Labels
epic 💪 This issue is an epic on our product roadmap Status: In Progress Work is in progress

Comments

@pettinarip
Copy link
Member

pettinarip commented May 18, 2022

This is an epic that makes up part of the ethereum.org Q2 roadmap

Description

Our codebase is rapidly increasing in size and in contributors. This brings challenges to reliably keep the code organized, avoid code duplication, and review code.

To help alleviate these challenges, we’ll be incorporating TypeScript into our codebase. We believe having a strongly typed language will reduce bugs, improve code quality, increase productivity and allow us to scale (both our codebase and our developer community) better in the long term.

This effort also pairs closely with implementing a UI library. Many UI library come with TypeScript support, which is worth taking advantage of.

Todo

How to contribute

This file is going to serve as a centralized ledger 🤪 to keep track of each js file that needs to be migrated.

If you’d like to help with this migration, please follow these steps:

  1. Leave a comment here asking which file you would want to work on.
  2. We assign you to the file (by adding your handle next to the file name in the list).
  3. Once you finish it (PR merged), we’ll mark it as done.
  4. Repeat!

📒 Complete list of js files to migrate

@Mousticke
Copy link
Contributor

Hi there, I would like to be assigned to these two components :

  • src/components/Accordion.js
  • src/components/Emoji.js

Thnak you very much

@pettinarip
Copy link
Member Author

Hey @Mousticke thanks for getting involved. As I mentioned in the Discord chat, I have assigned Emoji.js to @byhow already. Let me know if you still want to be assigned to Accordion.js and/or wait until the Emoji.js PR is merged. Or if you want to pick some other file/s.

Mousticke added a commit to Mousticke/ethereum-org-website that referenced this issue Jun 2, 2022
add typed props and style for the banner notification and switch from
js file to typescript react component

Refs: ethereum#6392
@Mousticke
Copy link
Contributor

Mousticke commented Jun 3, 2022

@pettinarip : After the #6543 PR is made, I'd like to migrate:

components

utils

Thank you :)

@pettinarip
Copy link
Member Author

@pettinarip : After the #6543 PR is made, I'd like to migrate:

Assigned! @Mousticke

Translation.js is being migrated in this PR #6540 you can jump into and review it and/or make any suggestion.

Mousticke added a commit to Mousticke/ethereum-org-website that referenced this issue Jun 3, 2022
add typed props and style for the accordion and switch from
js file to typescript react component

Refs: ethereum#6392
Mousticke added a commit to Mousticke/ethereum-org-website that referenced this issue Jun 3, 2022
add typed props for the card list and switch from
js file to typescript react component

Refs: ethereum#6392
Mousticke added a commit to Mousticke/ethereum-org-website that referenced this issue Jun 3, 2022
isPageIncomplete variable is truthy or falsy but the banner notification
component accept a boolean props. So we convert truthy or falsy to true
or false

Refs: ethereum#6392
@adi44
Copy link
Contributor

adi44 commented Jun 4, 2022

src/pages/layer-2.js
src/pages/run-a-node.js
src/pages/stablecoins.js
src/pages/staking
src/pages/staking/deposit-contract.js

I would like to work on this @pettinarip

Mousticke added a commit to Mousticke/ethereum-org-website that referenced this issue Jun 4, 2022
the card list components has been migrated to typescript
react component. For a given card list item, if the image is provided,
then the alt props must be passed as GastbyImage requires the alt props

Refs: ethereum#6392
@pettinarip
Copy link
Member Author

@adi44 assigned! and thanks!

Mousticke added a commit to Mousticke/ethereum-org-website that referenced this issue Jun 6, 2022
for typescript development, we migrate all javascript files to ts.

Refs: ethereum#6392
Mousticke added a commit to Mousticke/ethereum-org-website that referenced this issue Jun 6, 2022
Mousticke added a commit to Mousticke/ethereum-org-website that referenced this issue Jul 15, 2022
@Mousticke
Copy link
Contributor

Hi @pettinarip
I can migrate src/contexts/ZenModeContext.js just for the fun 😄

nikkhielseath added a commit to nikkhielseath/ethereum-org-website that referenced this issue Jul 16, 2022
@nikkhielseath
Copy link
Contributor

May I please convert the existing JS scripts to TS and make them work using ts-node?

@pettinarip
Copy link
Member Author

Hi @pettinarip I can migrate src/contexts/ZenModeContext.js just for the fun smile

Sorry @Mousticke, that file was already assigned, we had forgotten to put the handle next to it.

@pettinarip
Copy link
Member Author

May I please convert the existing JS scripts to TS and make them work using ts-node?

Good call @SNikhill are you talking about generate-heading-ids.js? I think it is the only script left to migrate. I have assigned it to you 👍🏼 thanks!

pettinarip added a commit that referenced this issue Jul 16, 2022
@Mousticke
Copy link
Contributor

@pettinarip the component WalletCompare doesn't exist anymore

@pettinarip
Copy link
Member Author

Good call @Mousticke, updated.

@pettinarip
Copy link
Member Author

@adi44 @andyGallagher Hey folks, as we are getting closed to finishing with the migration I wanted to touch base with you regarding the migration of the files that were assigned to you. I want to know if you are still planning on migrating them. If not, I can tackle them now and close the issue.

@adi44
Copy link
Contributor

adi44 commented Jul 21, 2022 via email

@pettinarip
Copy link
Member Author

Yeah.i couldnt do it on time

Thanks @adi44

Mousticke added a commit to Mousticke/ethereum-org-website that referenced this issue Jul 23, 2022
@nikkhielseath
Copy link
Contributor

nikkhielseath commented Jul 25, 2022

@pettinarip

I was migrating the script to Typescript and was wondering why tsc is not raising any errors.

I noticed that noImplicitAny was set to false therefore, most of the arguments and variables were being defaulted to any and the script was passing.

I wanted to verify if this was intentional to make the migration ease as this flag can give one a feeling of a correct TypeScript file even though it ain't.

--

I am setting it to true on my end for a stricter check.

nikkhielseath added a commit to nikkhielseath/ethereum-org-website that referenced this issue Jul 25, 2022
nikkhielseath added a commit to nikkhielseath/ethereum-org-website that referenced this issue Jul 25, 2022
@nikkhielseath
Copy link
Contributor

Sorry, I read the typescript doc just now. I wasn't aware that one existed.

@pettinarip
Copy link
Member Author

I am setting it to true on my end for a stricter check.

Yes, exactly @SNikhill. I try to that myself to avoid anys while I'm migrating a file. Once we finish with all the files, we are going to set that flag to true.

This was referenced Jul 26, 2022
@pettinarip
Copy link
Member Author

...and we are done! thanks to everyone who has contributed. Amazing job 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic 💪 This issue is an epic on our product roadmap Status: In Progress Work is in progress
Projects
None yet
Development

No branches or pull requests

10 participants