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

Consider using Flow or TypeScript in the Front-End Codebase #383

Open
maestromac opened this issue Aug 17, 2018 · 8 comments

Comments

Projects
None yet
4 participants
@maestromac
Copy link
Collaborator

commented Aug 17, 2018

@nickytonline commented on Sun Apr 29 2018

I'm not saying we need to do this, but yes I am a convert of static typing in JavaScript, so I do have some bias on this.

Having worked on several projects with large JS codebases, I can definitely say that it eliminates a lot of silly mistakes, improves dx and it gives developers a clearer view of what contracts and shapes of things are in the codebase. I've even written about it in the context of TypeScript.

The reason why I'm proposing this is twofold. The first is everything above, the second reason is it may (no guarantees) pique developers interest in contributing to dev.to OSS on the front-end more than say on a project that does not use TypeScript or Flow.

I've used Preact with TypeScript and the support seems solid since their last release, but for Flow, I'm not sure as I haven't really used Flow. I threw out this question to the Twitterverse, https://twitter.com/nickytonline/status/990768742178152448.

A third proposal, if this was a no go for everyone, is you can still get some type checking from TypeScript if you're using VS Code without event having a whole TypeScript build pipeline set up. If you add a // @ts-check comment at the top of the file, you get type checking. See https://code.visualstudio.com/docs/languages/javascript#_type-checking.

I was wondering what peoples' thoughts are on this?


@benhalpern commented on Tue May 01 2018

I"m in favor of this. And I think we're getting towards the end of a sprint where I and we collectively haven't been in the mindset to go back to the drawing board, but we're getting there now. This is definitely a convo I'd like to be having.


@nickytonline commented on Mon Jul 09 2018

@ben, this is probably something good to discuss before open sourcing the code base.

Looking at Flow and TS, I would probably lean more towards TypeScript. Not just because it's what I've been using professionally for quite some time, but because I think the ecosystem of types is larger and it has more adoption/tooling.

For reference, my blog post, Consider Using TypeScript mentions some fairly large/popular projects using TS, e.g. Slack, MobX, LinkedIn, RxJS etc.

Even though the new Preact components are currently just JS, you can go into a hybrid mode and slowly convert things to TS while still having JS live in TS land as valid JS is valid TS. This is what we do at the moment with a large project that we're converting slowly to TS.

This could also be a good way to have some live sessions about contributing to the code base. Maybe a few sessions about TS.


@nickytonline commented on Thu Aug 16 2018

@maestromac, when you have a chance, can you migrate this issue to the public repo? No rush as I'm off for another week. Thanks.

@stale

This comment has been minimized.

Copy link

commented Jan 7, 2019

Thanks for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue in 7 days. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If this issue still requires attention, please respond with a comment. Happy Coding!

@stale stale bot added the stale label Jan 7, 2019

@nickytonline

This comment has been minimized.

Copy link
Contributor

commented Jan 7, 2019

This clearly did go stale, but I think it's something to still keep on the radar. I'm happy to help with this initiative if we decide to go ahead.

@stale stale bot removed the stale label Jan 7, 2019

@stale

This comment has been minimized.

Copy link

commented Apr 7, 2019

Thanks for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue in 7 days. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If this issue still requires attention, please respond with a comment. Happy Coding!

@stale stale bot added the stale label Apr 7, 2019

@nickytonline

This comment has been minimized.

Copy link
Contributor

commented Apr 7, 2019

Not stale.

@stale stale bot removed the stale label Apr 7, 2019

@nickytonline

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2019

Popping a link to the discussion on dev.to here as well, https://dev.to/nickytonline/dev-to-with-a-typescript-or-flow-frontend-codebase-1n33

@nickytonline

This comment has been minimized.

Copy link
Contributor

commented May 18, 2019

Based on the comments in the post I made above, I've started to work on setting up the front-end code base for TypeScript (TS). I'm not in a state to PR this yet, but for those interested, the branch is here, https://github.com/nickytonline/dev.to/tree/move-to-typescript.

Here's where I'm at:

  • I converted the Search components to TypeScript
  • eslint configured for TS (still some things I'm ironing out)
  • Fixed some types that we'll be consuming, i.e. developit/linkstate#21 (PR waiting for review)
  • webpacker has a set up for TypeScript (TS), https://github.com/rails/webpacker/blob/master/docs/typescript.md, but I've opted to go with Babel 7+ with the TS preset. Using the TS preset allows the project to still benefit from Babel and it's ecosystem.
  • since I've gone with Babel, type checking when doing development will be an npm script that just runs tsc in watch mode.
  • For releases, the type checking will be another step in Travis CI.
  • I have yet to convert jest and Storybook to use TS.

Also, it'd probably be good to get this PR in first, #2034. I can give you a hand on this if you want @maestromac.

@rhymes

This comment has been minimized.

Copy link
Collaborator

commented May 18, 2019

Hi @nickytonline, thanks for the update and for the proof of concept! Your explorations are always appreciated!

I'm a little bit scared by the on off switch on TS to be honest because it will result (albeit temporarily) in 3 code bases (js legacy, jsx files and type script files) instead of 2 without actually improving the code. It might also burden the contributor: they have to be familiar with two languages (ES5 and ES2015) and TS tooling.

It's like we're adding a new tool that might pay off or not (after all we don't know it yet, right? though it's likely to help) instead of going the way of decreasing the technical debt first.

The Preact code is quite isolated, tested, mostly self contained (it's grafted in the pages that need the components). It definitely needs some love in the form of fixing the errors raised by eslint and prettier (just change any file and try to commit it and you'll see) but it's quite solid.

The legacy JS in constrast has no tests, it's written in ES5 and is included in every page. It works because the team is skilled but well, it's thousands and thousands of lines :)

It's like we're adding alternatives instead of decreasing debt. In a sense it's like taking an app that has a bug, aim to rewrite from A to B and what we're left is B with the bug instead of a better A.

I like new toys as much as the next person and I'm not saying I'm against TS tout court, I just think we should be gradual at how we arrive to it if the team decides to go that way and aim to decrease tech debt first. I know it's a mammoth effort but I don't see the rush either, there are no silver bullets after all ;-)

Please let me know what you think and if you would be in favor of a step by step approach. Also check @benhalpern comment here: https://dev.to/ben/comment/a4eh

Anyhow, thanks for the food for thought and for your time, it's always great to exchange ideas with you and seeing new takes on things.

A side note: if I opened a JSX file in my VSCode I saw a lot of warnings and errors (the ones you get also with the precommit hook) and eslint and prettier seem to be fighting each other (I literally see one pass changing the code in one way and the second pass changing the code in a different way), could it be an error in the order they are run or something wrongly setup on my end? 😅

@nickytonline

This comment has been minimized.

Copy link
Contributor

commented May 18, 2019

@rhymes, Here's a bit more context on the proof of concept (POC). This will allow you to use TypeScript (TS) in the project, but you are not forced to. It also only applies to the modern portion of the application, e.g. app/javascript. Also, whenever we do go forward with this (hopefully yes 😉), I'm happy to help pair with devs contributing to the frontend codebase.

I get the fracturing, but it's not as dire as it sounds. It's all JS at the end of the day. 😉

In regards to modernizing the front-end, e.g. porting the ES5 codebase to ESNEXT, I'm happy to help on that front as well. We can probably make a separate issue for that. As we port things we can also add tests.

In regards to the prettier/eslint battle, that is definitely a bug and most likely my fault when I configured it initially in the codebase way back when. That deserves its own ticket. Happy to tackle that one too or assist if someone else wants to take it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.