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

ADR for Typescript Guidance #412

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions docs/0003-typescript-guidance.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# Introducing TypeScript

## Status
Accepted

## Context
TypeScript is a strongly-typed superset of JavaScript that adds optional static type checking, class-based object-oriented programming, and other features to JavaScript.
Here are some of the advantages of TypeScript over JavaScript:

### Type safety
TypeScript helps catch errors at compile-time instead of runtime by adding type annotations to variables, functions, and classes. This can help prevent errors that might occur when dealing with large codebases (particularly when engaging in large refactorings).
### Better tooling support
TypeScript has better tooling support than JavaScript, with features like code navigation, auto-completion, and refactoring tools in popular code editors like Visual Studio Code.
Copy link
Member

Choose a reason for hiding this comment

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

[suggestion] Including a mention of generating API documentation from TypeScript might be worthwhile? i believe there's improved tool for auto-generated TypeScript docs compared to JSDocs documentation generation. (That said, I don't have much personal experience here.)

### Improved code organization
TypeScript's class-based object-oriented programming model allows developers to organize code in a more structured and maintainable way.
### Easy adoption
TypeScript is a superset of JavaScript, which means that any valid JavaScript code is also valid TypeScript code. This makes it easy for developers to adopt TypeScript gradually, without needing to rewrite their entire codebase.
### Community support
TypeScript has a growing community of developers who contribute to its development, create libraries and tools, and provide support to other developers. This makes it easier for developers to learn and adopt TypeScript.

## Decision
### Alternatives Considered
#### Flow
Flow, developed by Facebook, accomplishes many of the same things as TypeScript, albeit as an annotation system bolted on to Javascript rather than as a separate language. While it theoretically has an easier learning curve, its popularity has been eclipsed by TypeScript over time.
#### Plain JavaScript with more JSDoc documentation
While JSDoc strings can annotate functions argument types and even complex objects, it doesn't have the same level of linting tooling that TypeScript does.

### Target Goals
Prioritize using TypeScript in the following places:
* New code files
* Existing API endpoints (and their payloads)
Copy link
Member

Choose a reason for hiding this comment

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

[curious] If we add types for expected responses from API endpoints, I might be curious how those would interact, if at all, with Pact contract testing if/when we adopt contract testing more broadly throughout the platform. For example, in a world with contract testing via Pact, would we need to implement TypeScript types for API responses?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I have seen of contract testing, you wouldn't need to provide TypeScript types, but they would surely help in writing contract tests. Overall, I would consider contract testing and TypeScript types to be supplementary means of ensuring that the api contracts are adhered to.

* React Context/Redux state objects
* Components or Functions take a lot of parameters, or use parameters that are themselves complex objects
Copy link
Member

Choose a reason for hiding this comment

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

[suggestion] I might also include types around things like React context values, as well.


Less important are:
* React components without complex input objects (especially since they are often adequately served with PropTypes)
Copy link
Member

Choose a reason for hiding this comment

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

[curious] I'm wondering whether having to decide between adding types via PropTypes or TypeScript types being dependent on something relatively subjective (i.e., define "complex") could lead to some potential confusion around when to use which, when, and why. In some situations, we may want/need both. For example, in this Paragon PR to add types to all components, both TypeScript types and PropTypes are used together. Granted, Paragon's Gatsby documentation website relies on the PropTypes object to generate the props API.
image

* Test code

### Migration Strategy
Copy link
Member

Choose a reason for hiding this comment

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

[aside] We may want to consider when it makes sense to get our TypeScript supported alpha release of @edx/frontend-build merged into master to be published on the main distribution channel for NPM. At what point will we feel confident that merging alpha into master won't cause any regressions for non-TypeScript repos, in their build/test/lint/release pipelines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my mind, the safe time to do it is when all of frontend-build's consuming repos either
A) Are consuming frontend-build's alpha release with TypeScript in place
B) Have at least a working draft PR of consuming the alpha release with all the pipeline checks passing.

Does that sound stringent enough (and practical given the number of consumers)?

While migrating Javascript code to TypeScript code is relatively straightforward, the initial introduction of TypeScript into a code base can involve some unexpected wrinkles. While we will
work to streamline our infrastructure to make this easier over time, we advise introducing TypeScript in a minimalist fashion initially, by introducing or migrating a single file into TypeScript (or even a single function within a file).

The reasons for this are practical:
* The most difficult part of the initial process of introducing TypeScript into a Javascript codebase is harmonizing with the existing build, test, and lint configuration.
* The smaller the initial effort of introducing TypeScript into a codebase, the easier it is to actually implement around more pressing feature work.

## Consequences
### Benefits
* Code that requires heavy contracts, whether that's functions/components with lots or parameters, or complex objects returned from backend API's, will become much more comprehensible and easier to work with in a modern programming IDE.
* Because TypeScript is a superset of Javascript, the code does not need to be migrated all at once, but can be updated to TypeScript during the course of regular feature work.

### Challenges
* Migrating to TypeScript falls into the category of Tech Debt, which is difficult to get around to around the demands of mission-critical feature work and bug fixes.
* There can be a bit of a learning curve around using TypeScript, particularly for Javascript developers who don't have prior experience with strongly-typed languages.
Copy link
Member

Choose a reason for hiding this comment

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

It might be worth adding an "Alternatives considered" section with some points such as:

  • Flow. It's another popular type system for JS; has largely been outpopularized by TypeScript.
  • Continuing to use and formalize JSDoc docstrings. For example, these can give you some of the same type hinting benefits in IDEs as TypeScript, but is not as feature rich.
  • Do nothing and stick with regular ol' JavaScript.


## Follow-up Questions
### Should we still use...
#### React PropTypes
For repositories with PropTypes already in place (and related eslint checks), it likely still makes sense to keep using PropTypes alongside TypeScript for now. Additionally, some tooling currently in use(such as Gatsby documentation) relies on PropTypes.

## References
* https://www.typescriptlang.org/