-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
[SIP-36] Proposal for standardizing use of TypeScript #9101
Comments
Agree with this direction. |
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. |
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. |
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. |
I agree with using |
Would be great to use the same build/lint/test config that's used in |
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:
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:
Well, both Reason and Elm are used by much larger amounts of people, so that argument does not apply to them. |
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.
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. |
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. |
We could enforce strong typing via a combination of |
@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. |
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 |
as long as the plugin maintainers manage their own build tools and configure their own repositories to compile to |
@kristw 💯I'm not suggesting adding any more work to main plugins repo as there's already plenty to be done there. |
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 |
* refactor(frontend): move utils to typescript (#9101) * refactor(frontend): don't export interfaces * test(frontend): update types and test for isValidChild
* refactor(frontend): move utils to typescript (apache#9101) * refactor(frontend): don't export interfaces * test(frontend): update types and test for isValidChild
[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 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
The text was updated successfully, but these errors were encountered: