[SIP-36] Proposal for standardizing use of TypeScript

[etr2460]

[SIP-36] Proposal for standardizing use of TypeScript

Motivation

Since SIP-9 over a year ago, TypeScript has been a part of the Superset repo, but until now there has been no formal recommendation for using TypeScript throughout the Superset codebase. We've seen significant improvements in code quality and a decrease in bugs by requiring Python type hints, and therefore propose to enforce a similar requirement for TypeScript. By formally adopting TypeScript as the preferred frontend language within Superset, we expect to see a decrease in bugs and increase in developer productivity.

Proposed Change

CONTRIBUTING.md will be updated with the following requirements:

• New frontend files should be written in TypeScript
• When modifying old functions/components, migrating to TypeScript is appreciated, but not required
• Additionally, examples of new functions/components in TypeScript and migration PRs will be provided alongside these requirements

New or Changed Public Interfaces

N/A

New dependencies

N/A, TypeScript is already integrated within Superset

Migration Plan and Compatibility

N/A

Rejected Alternatives

Flow

Flow is an alternative typing system for Javascript, but it is only used by 35% of people (https://2019.stateofjs.com/javascript-flavors/other-tools/) compared to
TypeScript with > 60% (https://2019.stateofjs.com/javascript-flavors/typescript/)

No Typing

We have rejected no JavaScript typing as an alternative because of the reasons mentioned above.

to: @mistercrunch @rusackas @graceguo-supercat @nytai @kristw

[kristw]

Agree with this direction. @superset-ui is already 100% typescript. New plugins should be written in typescript as well.

[rusackas]

Just one small note of devil's advocacy: I agree with the "decrease in bugs" advantage, but I'm not 100% sure about the "increase in developer productivity," since by nature TS adoption requires devs in the community to know or learn TypeScript, which adds a layer of complexity and overhead. That said, I do think it's worth the trouble. As long as the examples cover most common use cases, and TS standards set at an approachable level in PR reviews (i.e. pushing things forward, rather than demanding perfection), this has my support.

[etr2460]

Fair point about developer productivity, my perspective is that any short term decrease in velocity due to devs in the community ramping up on TS will be quickly overshadowed by an increase in productivity due to the inherent safety types provide while developing and bug fixing.

[nytai]

I've found some of the lint rules and general use of tslint to be less friendly than the linting rules for regular js files, I've opened this #8865 to address the issue.

[kristw]

I agree with using eslint for both js and ts

[nytai]

Would be great to use the same build/lint/test config that's used in @superset-ui

[metaperl]

Given that superset-ui is already 100% javascript, it appears this SIP is more about "standardizing" (enforcing type features of) the use of Typescript than deciding on something that meets the objectives stated in the overview.

I.e, the overview stated:

We've seen significant improvements in code quality and a decrease in bugs by requiring Python type hints, and therefore propose to enforce a similar requirement for TypeScript.

And so it seems that there is a desire for type-safety. And it appears that such type-safety is optional in Typescript as it is in Python.

and the question arises why not use Reason or Elm so that the language itself enforces type-safety?

Also, from the overview, I notice that Flow was rejected because:

it is only used by 35% of people (https://2019.stateofjs.com/javascript-flavors/other-tools/) compared to TypeScript with > 60% (https://2019.stateofjs.com/javascript-flavors/typescript/)

Well, both Reason and Elm are used by much larger amounts of people, so that argument does not apply to them.

[nytai]

While reason and elm are excellent languages to start new projects in and have a lot of benefits in terms of type safety I don't think it makes sense to introduce those technologies to superset (yet). Typescript is merely a superset of javascript, thus all javascript is valid typescript, the same cannot be said about elm or reason. Reason and elm are compiled languages with compiler checks and the code that's run in the browser can be quite different than the code written by the developer. From a strictly type safety perspective, you are correct those 2 languages provide much greater safety than typescript. However, introducing these technologies would mean a significantly change in the developer experience and workflow. At this point it's not reasonable to propose porting the existing codebase to reason or elm, nor does it seem reasonable to support 2 distinct developer experiences for the same code.

Well, both Reason and Elm are used by much larger amounts of people, so that argument does not apply to them.

I'm not sure this is accurate, the number of developers writing typescript exceeds the number of developers writing reason and elm by orders of magnitude. The stack overflow developer survey does not even include those languages in their rankings.

[metaperl]

I'm not sure this is accurate, the number of developers writing typescript exceeds the number of developers writing reason and elm by orders of magnitude.

What i was saying is that the argument against Flow (lack of user base) does not apply to Reason or Elm. Because of the user base. But, perhaps it does.

I wasnt comparing the userbase of Reason/Elm to Typescript, but to Flow.

[kristw]

And it appears that such type-safety is optional in Typescript as it is in Python.

We could enforce strong typing via a combination of strictXXX flags in tsconfig and eslint rules.

[nytai]

@metaperl yes, it seems facebook has stopped pushing flow recently in favor of reason. While I personally love ocaml's type system (used by flow and reason) and think it's far more sophisticated that typescript's, reason lacks the maturity and community that typescript has gained in recent years.

I think getting wider adoption of typescript in the project should be the first goal, then we can consider increasing strictness of typing.

We could experiment using elm/reason for some of the chart plugins, as that code is quite self-contained.

[etr2460]

FWIW, Facebook has stopped pushing flow but has started adapting TypeScript in some projects (Jest for example).

Using elm or reason in the chart plugins seems like a good idea, if someone wanted to implement their new plugin in a non JS/TS language

[kristw]

Using elm or reason in the chart plugins seems like a good idea, if someone wanted to implement their new plugin in a non JS/TS language

as long as the plugin maintainers manage their own build tools and configure their own repositories to compile to js to be consumable by incubator-superset. The main plugins repo currently does not provide support for these out of the box.

[nytai]

@kristw 💯I'm not suggesting adding any more work to main plugins repo as there's already plenty to be done there.

[etr2460]

This SIP has passed with 6 binding votes: https://lists.apache.org/thread.html/rf293da46afad231186ff6750095a6cd25614bdc72647389a40c9604d%40%3Cdev.superset.apache.org%3E

I'll follow up with changes to CONTRIBUTING.md and example PRs for migrating your functions/components prior to closing the issue.