Road to TypeScript

[jelhan]

The goal is shipping types from Ember Bootstrap. To ease maintenance, we should migrate Ember Bootstrap itself to TypeScript and Glint.

This is a noticeable amount of work. To not get lost while doing it and profit from it as soon as possible, I think we need doing it incrementally. This issue defines a roadmap to TypeScript for Ember Bootstrap and will allow us to track progress.

I'm not a TypeScript expert. Would love a review on this draft from people having more experience migrating bigger Ember addons to TypeScript.

1. Prerequisites

To ease migration and long-term maintenance, we should type the dependencies first:

• @ember/render-modifiers
• ember-concurrency
• ember-element-helper
• ember-focus-trap (Publish TypeScript types josemarluedke/ember-focus-trap#86)
• ember-on-helper (Publish TypeScript types buschtoens/ember-on-helper#381)
• ember-popper-modifier (Publish TypeScript types adopted-ember-addons/ember-popper-modifier#832)
• ember-ref-bucket (local types available, upstream tracked here Please supply typescript types lifeart/ember-ref-bucket#10)
• ember-render-helpers
• ember-style-modifier (Convert to TypeScript and publish types (including Glint) jelhan/ember-style-modifier#210)
• rsvp (dependency dropped in Drop RSVP dependency #2055)
• tracked-toolbox (TypeScript Support tracked-tools/tracked-toolbox#38)

2. Setup

Goal is to setup TypeScript and Glint to allow incremental typing of utilities, components and helpers

• Setup TypeScript and Glint (Add initial typescript configuration to addon project #2092)
• Setup type testing infrastructure (Add initial typescript configuration to test-app project #2096)

3. Adopt TypeScript incrementally

Incrementally adopt TypeScript for components. Tests for the types should be added when migrating a file.

3.1 Convert shared utils and helpers to TypeScript

Utils and helpers should be converted to TypeScript in a first step as they are likely needed by many components. All utils and helpers may be done in one PR or splitted over multiple PRs.

• addon/utils/*
• addon/helpers/*

3.2. Convert components to TypeScript

Components should be converted to TypeScript incrementally to speed up review. We should start with those components, which are not depending on any other component. Components having a dependency on another component should only be picked up when that other component is converted to TypeScript.

• <BsAccordion> and <BsAccordion::Item>
• <BsButton> (Convert BsButton to TypeScript #2093)
• <BsButtonGroup> and <BsButtonGroup::Button> (dependency: <BsButton>)
• <BsCarousel> and <BsCarousel::Slide>
• <BsCollapse> (Convert BsCollapse to TypeScript #2095)
• <BsDropdown>, <BsDropdown::Button>, <BsDropdown::Menu>, <BsDropdown::Menu::Divider>, <BsDropdown::Menu::Item>, <BsDropdown::Toggle> (dependency: <BsLinkTo>) ([WIP] Convert BsDropdown to TypeScript #2109)
• <BsForm>, <BsForm::Element>, <BsForm::Element::Control::*>, <BsForm::Element::Errors, <BsForm::Element::FeedbackIcon>, <BsForm::Element::HelpText>, <BsForm::Element:Layout::*>, <BsForm::Element::Label>, <BsForm::Element::Legend> (dependency: <BsButton>)
• <BsLinkTo> (Convert BsLinkTo to TypeScript #2099)
• <BsModal>, <BsModal::Body>, <BsModal::Dialog>, <BsModal::Footer>, <BsModal::Header> (Convert BsModal related classes to TS #2115)
• <BsModal::Simple> (dependency: <BsModal>
• <BsNav>, <BsNav::Item> (dependencies: <BsDropdown>, <BsLinkTo>)
• <BsNavbar>, <BsNavbar::Content>, <BsNavbar::Toggle>, <BsNavbar::Nav>, <BsNavbar::LinkTo> (dependencies: <BsLinkTo>, <BsNav>)
• <BsPopover>, <BsPopover::Element> (Convert Tooltip/Popover to TypeScript #2112)
• <BsSpinner> (Convert BsSpinner to TypeScript #2108)
• <BsTab>, <BsTab::Pane> (dependency: <BsNav>)
• <BsTooltip>, <BsTooltip::Element> (Convert Tooltip/Popover to TypeScript #2112)

4. Publish types

• Publish the types and include them in next release

To consider

Types should take Bootstrap theming capabilities into account. For example, a Bootstrap theme can customize the supported types for a button. Therefore @type argument of <BsButton> should be typed as string rather than assuming only the variants supported by Bootstrap's default theme ("primary" | "secondary" | "tertiary" | "danger" | "warning" | "info" | "light" | "dark" | "link").

Follow-up activities

There are some follow-up activities, which would be unblocked by this. But I think we should keep them out and focus on those tasks, which make a difference to the consumers of Ember Bootstrap.

• Convert docs app to TypeScript
• Convert remaining tests to TypeScript
• Use TypeScript types for API docs and clean-up duplication with existing JSDoc

Open question

• Should we support types published by ember-source itself and types from DefinitelyTyped? Or do we limit support to native types from ember-source to ease testing and maintenance? Supporting types from DefinitelyTyped may require fixing some incompatibilities of those with types from ember-source upstream.

[SanderKnauff]

One thing I noticed when looking at Typescript is that Yuidoc does not seem to support ts files.
The project would probably need another tool to generate the API docs

[jelhan]

One thing I noticed when looking at Typescript is that Yuidoc does not seem to support ts files. The project would probably need another tool to generate the API docs

Do you have details on that? I tried running YUIDoc on a simple TypeScript file and it seemed to work fine. Of course not picking up any information from TypeScript types. But maybe my test case was just too simple?

I run yuidoc with --extension '.ts' flag as it by defaults only picks up files having .js extension. That was the only configuration I provided.

[SanderKnauff]

One thing I noticed when looking at Typescript is that Yuidoc does not seem to support ts files. The project would probably need another tool to generate the API docs

Do you have details on that? I tried running YUIDoc on a simple TypeScript file and it seemed to work fine. Of course not picking up any information from TypeScript types. But maybe my test case was just too simple?

I run yuidoc with --extension '.ts' flag as it by defaults only picks up files having .js extension. That was the only configuration I provided.

Aha! I had missed that option in Yuidoc. I've tried the same with --extension '.js,.ts' and it matches your findings.
It indeed does not pick up most information that can be inferred from Typescript, so we would have to keep documenting the @class, @namespace and @component in the JSDoc.

I notice that in some places we are already specifying the wrong value in @extends, namely using @extends Ember.Component where a Glimmer component is used. I think ideally, a doc tool would infer that information from the types and imports...

[simonihmig]

One thing I'd would suggest we take a decision on is whether we want to do this first, or #1980, because the setup and overall boilerplate differs a lot. So if we do #1980 first, then we wouldn't need to get TS setup for a v1 addon. Also migrating to a real monorepo as part or prerequisite to #1980 might make dependency management easier, also with regards to TS...

Regarding Yuidoc, I have used TypeDoc before, and while it is not perfect (just as Yuidoc), it does the job (including picking up all type information), so we might want to do that!?

Therefore @type argument of should be typed as string rather than assuming only the variants supported by Bootstrap's default theme

Not sure I agree with that. Tight typing can become very handy when working with Glint in your editor, otherwise you wouldn't get any auto completion here. I think it should be possible to let users augment some interfaces if they want to tweak their types, which we can document somewhere.

[SanderKnauff]

One thing I'd would suggest we take a decision on is whether we want to do this first, or #1980, because the setup and overall boilerplate differs a lot. So if we do #1980 first, then we wouldn't need to get TS setup for a v1 addon. Also migrating to a real monorepo as part or prerequisite to #1980 might make dependency management easier, also with regards to TS...

I think it is a good call to first convert the repository into a V2 addon, especially with the difference in Typescript setup between V1 and V2 addons. I'm currently performing an atttempt to do so on https://github.com/SanderKnauff/ember-bootstrap/tree/v2-addon, following the V2 addon porting guide.

I'm not sure how long this will take, as I also need to document the required changes an addon consumer would have to make to upgrade between the current version of e-bootstrap and a v2 variant.

[jelhan]

I converted ember-style-modifier as a v1 addon to TypeScript. It was straight forward thanks to the official TypeScript support in recent Ember CLI blueprint.

What I have done:

1. Upgrade to latest Ember CLI blueprints using ember-cli-update
2. Run ember init --typescript in the project to convert the project to TypeScript
3. Reset all changes done by ember init, which were not related to TypeScript
4. Set up Glint (not fully done by Ember CLI blueprints yet)
5. Convert files of addon to TypeScript
6. Convert tests to TypeScript

Ember CLI takes care of (nearly) all boilerplate. Beside doing some adjustments for Glint (e.g. switching lint:types script in package.json) everything is ready out of the box. It was easy to spot, which changes done by ember init --typescript were required and which one should be reset.

I think we should not overestimate the additional work if setting up TypeScript before converting to a v2 addon. The amount of work is likely the same as if converting a v2 addon to TypeScript.

You can find the results in jelhan/ember-style-modifier#210. The commit history tells the story a little bit. The first commit (jelhan/ember-style-modifier@2d660c9) is the results of ember init and resetting unintended changes done by it.

[vlascik]

I did #1861 a year ago, most of the typing for glint should already be done. It needs someone to pick it up for couple of hours and get it through the finish line, but most of the menial work is already there.

Might be a decent enough starting point for a full rewrite to TS.

[SanderKnauff]

We could use #1861 as a band-aid solution for adding initial Glint support, and then using those types as a starting point for the actual migration.

@jelhan do you think it is worth it to finish that PR before we get the actual TS migration done?
This does mean that we have a set of public types that people will start to depend on. Those might change when we get more 'correct' types that are generated from the sources themselves...

[jelhan]

I feel there is a high risk of wrong typing if we write the types independently from the source code. We should at least convert the tests to TypeScript and Glint to have some testing for the types.

I don't have enough experience to estimate the overhead of manually writing types in an additional first step. Not sure how much that speeds up conversion in the second step.

If shipping manual maintained types, we should mark them as beta / preview and exclude them from our general SemVer commitment. That gives us the freedom introducing breaking changes to the types when converting to TypeScript in a second step.

Overall I feel it's primarily a decision of whoever is willing to invest the time and do it. 😄

[vlascik]

In my experience rewriting into TS is not as easy as it looks - once you start typing you'll realize you also want to use glimmer components and things like @tracked everywhere and it quickly snowballs from there - there are subtle differences in usage and timings, which will break your tests, so you'll also have to fix those. And write new ones. Oh, also APIs will probably have to change too, so GLHF.

On an addon this size it's probably weeks worth of effort. And given how much active this addon is (or rather isn't) I would put this into "not going to happen anytime soon" category, unless someone really steps up.

So as I see it, the choices are:

1. wait for a miracle to happen, and have no types maybe for years.
2. do the next best thing and get at least some types.

Glint types are a pure value add-on - even if they are wrong, they won't break your apps, and if you find an error in them, they're easy to fix anyway.

I wouldn't let the perfect be the enemy of good enough. The types alone improve DX so much it's worth some back and forth on fixing them even if they're not 100% on a first try.

Warn users that you can't ensure the types correctness without a full TS rewrite, and let them YOLO it from there. Worst case, they get a bunch of tsc errors, best case, they will improve types enough to start to migrate them into real TS components over time.

I took the PR as far as I could, but I simply don't use every component in this lib and can't tell if the types are correct for the ones I don't. It's also not always easy to see the intent behind the code I haven't written, and I don't have any more time to spend on it.

IMO someone needs to take that PR, try the types on their apps, fix what they find and leave the rest for the others to fix next time. Something > nothing, and there's a lot of value in having glint types alone. Arguably, end users probably do not even have to care about full TS rewrite anyway.

[jelhan]

In my experience rewriting into TS is not as easy as it looks - once you start typing you'll realize you also want to use glimmer components and things like @tracked everywhere and it quickly snowballs from there - there are subtle differences in usage and timings, which will break your tests, so you'll also have to fix those. And write new ones. Oh, also APIs will probably have to change too, so GLHF.

We have already done all that work as far as I'm aware. @SanderKnauff done an amazing job in November and December addressing the remaining cases. Please let me know if we missed something.

[vlascik]

We have already done all that work as far as I'm aware. @SanderKnauff done an amazing job in November and December addressing the remaining cases. Please let me know if we missed something.

Alright, I missed that, good to know. Still don't think that after a year of noone being able to even review/fixup the types, someone's going to write them from scratch any time soon. (Also I don't get why would anyone want to rewrite the same 7000 lines of boilerplate if they don't have to, but what do I know).

Anyway, it already works for me, so 🤷‍♂️.

[simonihmig]

Hey @vlascik, let me first say that I feel quite bad that we didn't give you very clear feedback on the general direction of your PR. I was for a while torn apart whether we should accept it or not. But I ultimately think it was the right decision to not merge it. That doesn't mean I/we don't appreciate all the effort you invested into it, I/we do!

But the initial contribution is one thing, and we could have just merged it or improved it and then merge. The cost of that would have been relatively low. But the actual cost would have been the obligation to maintain it. And if there is one thing I learned from this project that I started - what? - like 10 years ago or so, is that the effort for adding new features is just a fraction of the overall effort required to maintain such an OSS project.

So long story short, I think the only option to introduce and maintain types with a reasonable effort over time(!) is to migrate to native TS. And I think that's doable, at least we have some new momentum here and will try to get this done!

[vlascik]

@simonihmig In the end, typing a component = typing a component signature + class function parameters/return types. I did the signatures, and end users get no value from internal function types anyway.

Not sure how maintaining more types is going to be easier than maintaining less types. While doing the TS migration, you'll just end up having to write the same few thousands of lines of types again. I can't see a logic of doing that, so I feel like there's something lost in translation, do you think the types in component.d.ts files are somehow different from types you'd put into component.ts? You know you can also at any time just copy/paste the signature from component.d.ts into a component.ts file and go from there, right?

Anyway, as it makes no difference to me, I already monkey patch half the ember addons I use anyway, so what's one more - I'm out of this discussion 🤷‍♂️. Hope you'll succeed.

[simonihmig]

Maintaining more types is easier in this case, because you can be sure that the types are correct and stay correct. When receiving new contributions, I wouldn't be able to see easily if types need to change or are out of sync, when there is no way to test the types against the actual implementation.

You know you can also at any time just copy/paste the signature from component.d.ts into a component.ts file and go from there, right?

Yes, and I actually just did that in #2093, and it turned out that your onClick type was not quite right, if you compare yours to this one. Having the implementation in TS immediately surfaced that!

[Baltazore]

I've started working on BsSpinner here #2108