freeCodeCamp icon indicating copy to clipboard operation
freeCodeCamp copied to clipboard

TypeScript Migration

Open ShaunSHamilton opened this issue 2 years ago • 61 comments

Co-ordination

As discussed in the last contributors meeting we're looking to migrate our client to TypeScript.

No files will be assigned

PRs will be prioritised by quality, then by date opened

Guidelines

Take a look at the Codebase Best Practices - kebab-case file names

Small is beautiful

If a file can be migrated on its own, that's perfect. That makes the PR incredibly easy to review and minimises the chance we break anything.

PropTypes can go

None of the components in the client directories are intended to be consumed by JavaScript components - eventually everything will be tsx. With that in mind static type checking should be enough.

React Guidelines

So, we're all on the same page: https://github.com/typescript-cheatsheets/react#readme has some useful advice. Mostly the linters will warn us, but this is definitely worth a look.

un-typed packages

If the package does not come with types attached, then please npm i the definitions (if they exist) from DefinitelyTyped. Usually they're just published as @types/name-of-package.

Do Not Ignore Linting, Unless Absolutely Necessary

We hope to leverage the type-safety that comes with TypeScript. So, do not ignore the compiler's warnings/errors. If you are stuck, feel free to reach out for help.

Keeping Git History

Sometimes changing the file from <filename>.js -> <filename>.tsx? causes the original file to be deleted, and a new one created, and other times the filename just changes - in terms of Git. Ideally, we want the file history to be preserved.

The best bet at achieving this is to:

  1. Rename the file
  2. Commit with the flag --no-verify to prevent husky from complaining about the errors
  3. Refactor to TS for migration, in a separate commit.

Note: Editors like VSCode are still likely to show you the file has been deleted and a new one created. If you use the CLI to git add ., then VSCode will show the file as renamed in stage

Tests are running

We will not be merging any PRs with failing tests.

Files that should not be migrated (yet)

  • gatsby-node.js
  • gatsby-config.js and related
  • redux/*

To finish migrating the client, we would need additional tooling to handle gatsby-config, gatsby-node and files they import. This could either be an extra tsc compilation step or potentially via ts-node. However, I'm not sure if ts-node would integrate nicely with gatsby develop or gatsby build, so the compilation approach is probably easier.

Before going ahead with this, we should profile gatsby develop and gatsby build to make sure that the time to develop and build the site do not increase too much.

More guidelines may get added

As we do this, chances are we'll revise the approach. Any thoughts/comments/critiques are welcome, but I wanted to keep this straightforward for now.

List of Files To Migrate

  • [ ] /client/src/templates/Challenges/utils/worker-executor.js
  • [ ] /client/src/templates/Challenges/utils/worker-executor.test.js
  • [ ] /client/src/templates/Challenges/rechallenge/transformers.js
  • [ ] /client/src/__mocks__/react-i18next.js

ShaunSHamilton avatar Oct 08 '21 14:10 ShaunSHamilton

@ShaunSHamilton I can help. If there's anything available 😉

RobertoMSousa avatar Oct 08 '21 14:10 RobertoMSousa

@RobertoMSousa now it's back to usual, without assignments, as it says in the first post. PRs will be prioritized by quality and date opened.

In short, go for it with what you feel like with the non assigned files

ilenia-magoni avatar Oct 08 '21 14:10 ilenia-magoni

Should we keep the migration by component or can we merge them on a single PR? It's a little spam create 2 PR with so little changes 😅 https://github.com/freeCodeCamp/freeCodeCamp/pull/43783 https://github.com/freeCodeCamp/freeCodeCamp/pull/43784

But on the other hand, it's easier to review and keep track of possible bugs.

RobertoMSousa avatar Oct 08 '21 17:10 RobertoMSousa

@RobertoMSousa Don't worry, it's not spam. Small PRs, like the ones you've submitted, are great.

ojeytonwilliams avatar Oct 08 '21 19:10 ojeytonwilliams

Hello 👋

I've just opened a small PR with migration of remaining parts of the Landing component.

If I find some more time over the weekend, and there will still be something to do, I'll try to help :)

sitek94 avatar Oct 08 '21 20:10 sitek94

I'll also like to help with this. I can see a couple of PRs already. How do I know which unassigned files are still pending?

Pranav2612000 avatar Oct 10 '21 08:10 Pranav2612000

@Pranav2612000 You can use this site(https://tools.freecodecamp.org) to check if an unassigned file has an open PR or not.

Nirajn2311 avatar Oct 10 '21 08:10 Nirajn2311

Hey, I just tried working on throwers.js and I noticed that it's not being imported anywhere. Are there plans for this file, or should it be deleted?

Ethan-Edmond avatar Oct 12 '21 01:10 Ethan-Edmond

I will work on : /client/src/components/layouts/Default.js

AbdelilahRami avatar Oct 13 '21 14:10 AbdelilahRami

Hey, I just tried working on throwers.js and I noticed that it's not being imported anywhere. Are there plans for this file, or should it be deleted?

Hi @Ethan-Edmond. Yep, that looks to be obsolete code that can be deleted.

ojeytonwilliams avatar Oct 13 '21 15:10 ojeytonwilliams

Hi! I'll work on these files

 /client/src/components/Map/Map.test.js
 /client/src/components/Intro/Intro.test.js
 /client/src/components/Intro/index.js

aprilsourz avatar Oct 13 '21 17:10 aprilsourz

Hey, would love to take on /client/src/templates/Challenges/components/Test-Suite.js

mendaomn avatar Oct 14 '21 08:10 mendaomn

@mendaomn files are not assigned anymore, just go for it

An issue with the help wanted or first timers only label is open for contribution. The first comprehensive PR created will be reviewed and merged. We typically do not assign issues to anyone other than long-time contributors.

If you would like to contribute, and have not read the contributors docs, please do so here: https://contribute.freecodecamp.org/#/

If you have any issues with contributing, be sure to join us on the contributors channel, or on the contributors sub-forum

P.S. Abbiamo anche un canale italiano, e sitamo lavorando a tradurre freeCodeCamp in italiano! Ci piacerebbe avere anche te!

ilenia-magoni avatar Oct 14 '21 08:10 ilenia-magoni

Alright cool! I opened #43857 and may need some help if anyone is willing!

mendaomn avatar Oct 14 '21 09:10 mendaomn

Ill take on: /client/src/components/landing/index.js

StanleyDharan avatar Oct 14 '21 15:10 StanleyDharan

Sorry my company needs to review this PR prior to me pushing (were doing a hacktoberfest event internally), code is done. Should be able to re-open the PR in about a day or so once they check it for security risks

StanleyDharan avatar Oct 14 '21 20:10 StanleyDharan

@StanleyDharan I suggest you put it in a draft, if you want precedence on it, a closed PR is not counted

ilenia-magoni avatar Oct 15 '21 06:10 ilenia-magoni

Hey all, I want to contribute to some files. ping me if anything is available.

abhishek-geek avatar Oct 15 '21 11:10 abhishek-geek

I will take: /client/src/components/landing/Landing.test.js

JasmineSun33 avatar Oct 15 '21 14:10 JasmineSun33

@StanleyDharan I suggest you put it in a draft, if you want precedence on it, a closed PR is not counted

Were unable to push anything to public repo's that aren't on a "pre-approved" list, I initially by-passed our checks by accident. So i'm not able to even push a draft until I get approved by our open source folks 🙃

StanleyDharan avatar Oct 15 '21 16:10 StanleyDharan

Hi! I would love to contribute to the TypeScript migration. I currently don't have TypeScript experience is this suitable for a TS beginner?

oakj avatar Oct 16 '21 15:10 oakj

@oakj Whilst TypeScript is not all that different compared to JavaScript, I would not easily recommend using a migration as your first experience into TypeScript, unless you are already familiar with another strongly typed language.

There is nothing stopping you from trying, and you are more than welcome to get help from the freeCodeCamp community in your learning process. So, I will leave you with some available resources:

ShaunSHamilton avatar Oct 16 '21 18:10 ShaunSHamilton

@ShaunSHamilton Thanks the reply! Appreciate the resources also. Will definitely go through the contributor docs and typescript docs!

oakj avatar Oct 16 '21 18:10 oakj

@ShaunSHamilton I would like to work on migrating client>utils>challenge-types.js to TS, but I noticed that 2 variable has the same value in this file : const zipline = 3; const frontEndProject = 3;

is this intentional or a mistake?

hishamss avatar Oct 21 '21 21:10 hishamss

is this intentional or a mistake?

@hishamss This is intentional. If I correctly recall, zipline is a legacy challenge type. It may very well be removable now, but, when messing with the legacy stuff, it is often difficult to tell if something will break.

ShaunSHamilton avatar Oct 22 '21 10:10 ShaunSHamilton

Hi, Is it fine if I'm unassigned from these files? I don't think I will be able to work on them anytime soon.

 /client/src/templates/Challenges/classic/EditorTabs.js
 /client/src/templates/Challenges/classic/MobileLayout.js
 /client/src/templates/Challenges/classic/MultifileEditor.js

meronogbai avatar Oct 24 '21 20:10 meronogbai

Hi, can I help with a few files? I can take on the ones from the comment above, if it's easier. Thanks!

wingclover avatar Oct 25 '21 03:10 wingclover

@wingclover files are not assinged anymore, the migration of the files in this issue are up for grabs - you are free to work on it and create a PR

ilenia-magoni avatar Oct 26 '21 16:10 ilenia-magoni

Hey @ShaunSHamilton, Can I take on these files migration? /client/src/client/frame-runner.js /client/src/templates/Challenges/classic/MultifileEditor.js /client/src/components/landing/index.js

ashiskumar-1999 avatar Nov 12 '21 08:11 ashiskumar-1999

@ashiskumar-1999 please check that there are not already PRs open for those, but otherwise you are free to go with them, files are not assigned and are always up for grabs in a first come first serve basis

ilenia-magoni avatar Nov 12 '21 11:11 ilenia-magoni

Thanks, @ieahleen, Let me check then.

ashiskumar-1999 avatar Nov 13 '21 12:11 ashiskumar-1999

Hey @ieahleen, There are no PR open on the /client/src/client/frame-runner.js and /client/src/templates/Challenges/classic/MultifileEditor.js files right now. Can I work on this?

ashiskumar-1999 avatar Nov 14 '21 06:11 ashiskumar-1999

Hi @ashiskumar-1999 ,

just go ahead, there is no need to ask for permission

Ismailtlem avatar Nov 14 '21 10:11 Ismailtlem

@Ismailtlem @moT01 Hi there! I was wondering if we can think on a better way to check if there is an open PR for a certain file.

When I created this PR, and used the method suggested above (or somewhere else, I don't remember xD) to check for that, and there were none listed.

It was a fairly small contribution and I have no issue with it not being merged, that's fine!

Still, maybe some sync is needed. No need for permissions, but some coordination to see which files need to be migrated yet? :)

Cheers, keep the hard work! I really appreciate it since I got my first dev job thanks to FCC :)

kamatheuska avatar Dec 09 '21 20:12 kamatheuska

Hi @kamatheuska

I think that you can use this tool https://tools.freecodecamp.org/ to check if some file is included in a PR or not

Ismailtlem avatar Dec 10 '21 13:12 Ismailtlem

Yeah, I used that one, and no PR's were created at that time for the files migrated in my PR. I could have used wrongly, since I cannot find any docs on how to write a query for it.

kamatheuska avatar Dec 10 '21 14:12 kamatheuska

@kamatheuska The two PRs which migrated the same components were

  • https://github.com/freeCodeCamp/freeCodeCamp/pull/43966 landing/index.tsx (opened 22/10)
  • https://github.com/freeCodeCamp/freeCodeCamp/pull/43889 landing/landing.test.tsx (opened 15/10)

The important thing to remember about using the above mentioned tool is to use the original name of the file to search (not the migrated name): image

I hope this was not too inconvenient for you. We do try to respect all contributor's time and effort.

ShaunSHamilton avatar Dec 10 '21 14:12 ShaunSHamilton

That's awesome feedback, thanks! @ShaunSHamilton

kamatheuska avatar Dec 10 '21 15:12 kamatheuska

Why is it not possible to migrate format.js to .ts ?

Ismailtlem avatar Feb 19 '22 16:02 Ismailtlem

@Ismailtlem I think this includes the format.js file: https://github.com/freeCodeCamp/freeCodeCamp/issues/42256#issuecomment-882546081

ShaunSHamilton avatar Feb 19 '22 19:02 ShaunSHamilton

I'll work on migrating these files.

/client/src/templates/Challenges/utils/frame.js /client/src/tests/integration/handled-error.test.js

Dripcoding avatar Feb 25 '22 21:02 Dripcoding

has anyone run in to this issue while working on migrations?

"Cannot use import outside of module"

When I try to use commonjs, the linter complains about the types I've created. I have a feeling it's some sort of config issue but googling hasn't yielded much.

PR for reference: https://github.com/freeCodeCamp/freeCodeCamp/pull/45456

Dripcoding avatar Mar 19 '22 03:03 Dripcoding

That happens when the js file gets used by node. Unless the file has the .mjs extension or the package is declared as a module, you have to use require.

Typescript can be configured to emit require though: https://www.typescriptlang.org/tsconfig#module

ojeytonwilliams avatar Mar 19 '22 09:03 ojeytonwilliams

Hello, I would love to help contribute to this and will work on migrating these files now:

/client/i18n/configForTests.js /client/i18n/locales.test.js

alcaidet avatar Apr 01 '22 04:04 alcaidet

@Sembauke I can see a few more files from the description haven't been migrated. Are you sure you want to close this issue?

raditotev avatar May 20 '22 13:05 raditotev

It was done automatically :)

If you want to migrate more files, please do!

Sembauke avatar May 20 '22 13:05 Sembauke