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

Typescript #761

Open
6 tasks
eritbh opened this issue May 26, 2023 · 1 comment
Open
6 tasks

Typescript #761

eritbh opened this issue May 26, 2023 · 1 comment
Labels
code quality meta concerns related to releasing/maintenance/etc that don't rely on changes to extension code

Comments

@eritbh
Copy link
Member

eritbh commented May 26, 2023

This has been discussed some in the past on Discord, but I'm making an issue for it now to collect previous thoughts because some of the work I've been doing recently would benefit a lot from TS support and it's motivating me to revisit my old proof of concept for this.

Using TS in toolbox would be an incremental thing and would probably follow roughly these steps:

  • Add support for tsc as part of our build process (allowJs without any code change and make sure the build still functions)
  • Make sure our existing JS doc comments work with type checking (// @ts-check, checkJs)
  • Start converting files to TS proper
    • Start with files that are basically just free functions with good documented types, e.g. TBHelpers.js, TBApi.js
  • Figure out what we want to do about JSON types returned from Reddit (do we just make up our own in the return types of TBApi methods? does anyone else maintain a package we can pull in to get types for API responses? do we just give up and type everything as object? (please don't just give up and type everything as object))
  • Slowly work towards noImplicitAny and other stricter parsing options as we work through the rest of the extension
  • at some point replace our old broken docs build process to use typedoc or something

Specific areas where TS would be useful:

  • Editing module settings can be greatly improved with TS - the type inference system can be leveraged to tell you what properties are expected on settings of a certain type while you're typing out the object, and it can also be used to make reading/writing a misspelled storage key a build-time error
  • Anywhere we're passing around HTML/jQuery objects between functions - it's not always clear what functions expect an HTML string, a jQuery object, or either, even with the documentation we have in place now
  • Certain helper functions, particularly those that deal with arrays, would benefit from being described by template types, but jsdoc @template is cumbersome and we don't really use it much
  • Sharing types between the content script and background page will be easy and helpful for making sure our message passing is sound
  • Deepscan has helped with a lot of this already, but TS type narrowing is useful for eliminating certain dead code (e.g. using type information to identify when a branch can never be taken)
@eritbh eritbh added code quality meta concerns related to releasing/maintenance/etc that don't rely on changes to extension code labels May 26, 2023
@creesch
Copy link
Member

creesch commented May 26, 2023

old broken docs build process

Excuse me, but I fixed that at some point :P

Other than that, sounds like a plan. I'd add a step where we update our contributing docs where we state that we use typescript and link to a getting started resource. Just in the interest of making contributing still as accessible as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality meta concerns related to releasing/maintenance/etc that don't rely on changes to extension code
Projects
Status: Todo
Development

No branches or pull requests

2 participants