[archive] Fix usage with TypeScript 4.8+

[fcollonval]

Pull Request

📖 Description

Add typing to template variable to remove error seen in code consuming fast-foundation with TypeScript 4.8+

🎫 Issues

Fixes #6365

👩‍💻 Reviewer Notes

📑 Test Plan

✅ Checklist

General

• I have included a change request file using $ yarn change
• I have added tests for my changes.
• I have tested my changes.
• I have updated the project documentation to reflect my changes.
• I have read the CONTRIBUTING documentation and followed the standards for this project.

I'm not sure how to test for this. I force locally a TypeScript update to 4.9.5 and fixed the error mentioned in @microsoft/fast-foundation.

There are also error in @microsoft/fast-router but this does not fix them.

Errors in @microsoft/fast-router

src/configuration.ts(49,9): error TS2322: Type 'DefaultRouteRecognizer' is not assignable to type 'RouteRecognizer'.
  The types returned by 'recognize(...)' are incompatible between these types.
    Type 'Promise | null>' is not assignable to type 'Promise | null>'.
      Type 'RecognizedRoute | null' is not assignable to type 'RecognizedRoute | null'.
        Type 'RecognizedRoute' is not assignable to type 'RecognizedRoute'.
          Type 'unknown' is not assignable to type 'TSettings'.
            'TSettings' could be instantiated with an arbitrary type which could be unrelated to 'unknown'.
src/recognizer.ts(271,17): error TS18047: 'segmentA' is possibly 'null'.
src/recognizer.ts(271,33): error TS18047: 'segmentB' is possibly 'null'.
src/recognizer.ts(275,17): error TS18047: 'segmentA' is possibly 'null'.
src/recognizer.ts(275,33): error TS18047: 'segmentB' is possibly 'null'.
src/router.ts(201,13): error TS2322: Type 'unknown' is not assignable to type 'Router | null | undefined'.

⏭ Next Steps

[chrisdholt]

Thanks for this @fcollonval - do we have a repro of this I can check out? I recall us seeing a similar TS issue which disappeared with a different fix which wasn't specific to 4.8. If I can validate the issue, happy to take an update/fix assuming it makes sense.

[fcollonval]

Thanks @chrisdholt

We have seen that first when working on https://github.com/brichet/jupyterlab/tree/toolbar_webcomponent (uses TypeScript 5.0.4).

But I bumped TypeScript (to 4.9.5) in the components library we are using built on fast v1 to ease error reproducibility: https://github.com/jupyterlab-contrib/jupyter-ui-toolkit/actions/runs/5600572055/jobs/10243153138?pr=63#step:9:200

[chrisdholt]

Thanks @chrisdholt

We have seen that first when working on https://github.com/brichet/jupyterlab/tree/toolbar_webcomponent (uses TypeScript 5.0.4).

But I bumped TypeScript (to 4.9.5) in the components library we are using built on fast v1 to ease error reproducibility: https://github.com/jupyterlab-contrib/jupyter-ui-toolkit/actions/runs/5600572055/jobs/10243153138?pr=63#step:9:200

Thanks for this, sorry for the delay. I'm going to fork and pull your repo to try and reproduce this. I have a feeling it may not be the exact TS issue. I also could be wrong :)

[fcollonval]

Thanks a lot for taking the time @chrisdholt

[m-akinc]

Any updates? Also interested in using TS 4.8+.

[atmgrifter00]

Would it be good to create a couple of local types to consolidate some of these type concerns. Namely:

type DesignTokenType = string | number | boolean | BigInteger | null | Array<any> | symbol | {};

type CSSDesignTokenType = DesignTokenType |
        ({
              createCSS?(): string;
         } & Record<PropertyKey, any>);

Then you could do something like this everywhere:

public static from<T extends CSSDesignTokenType> ...

I'll note that the changes here (with or without my proposed changes) do unblock our migration to Typescript 4.8, which is required for our components to work in the current version of Angular.

[rajsite]

The TypeScript version upgrade is blocking us from performing Angular upgrades without having all our applications disable library type checking.

Angular 14 support ends in November and we would like to be able to migrate to Angular 16 which requires TypeScript >=4.9.3.

Ideally we can avoid falling out of supported Angular version usage so hopefully within the next week or two we would like FAST to be compatible with a newer TypeScript version so we can bubble up support through our libraries. @chrisdholt do you think that would be possible?

[chrisdholt]

The TypeScript version upgrade is blocking us from performing Angular upgrades without having all our applications disable library type checking.

Angular 14 support ends in November and we would like to be able to migrate to Angular 16 which requires TypeScript >=4.9.3.

Ideally we can avoid falling out of supported Angular version usage so hopefully within the next week or two we would like FAST to be compatible with a newer TypeScript version so we can bubble up support through our libraries. @chrisdholt do you think that would be possible?

We're focused on updating the latest alpha and beta versions, but it's requiring some changes to our testing libraries. The bulk of my effort for TS right now is going to focus on moving those versions forward towards stable and hopefully getting onto the latest TS version for alpha/beta versions so we can leverage TS's support for decorator metadata.

As it pertains to the archive branches - we can try to get some support there, but I would suggest starting to plan on how to move off those. Given the alpha/beta changes have been moving slower, we've continued to backport some fixes, but I don't see that going on forever. At some point, the code there is just difficult to maintain with backwards compatibility, especially given the amount of the time the FAST Components package has been deprecated. I'm keeping this open with the hopes we can get to updating the archive, but changes there are more difficult and time consuming as of now.

[chrisdholt]

The TypeScript version upgrade is blocking us from performing Angular upgrades without having all our applications disable library type checking.
Angular 14 support ends in November and we would like to be able to migrate to Angular 16 which requires TypeScript >=4.9.3.
Ideally we can avoid falling out of supported Angular version usage so hopefully within the next week or two we would like FAST to be compatible with a newer TypeScript version so we can bubble up support through our libraries. @chrisdholt do you think that would be possible?

We're focused on updating the latest alpha and beta versions, but it's requiring some changes to our testing libraries. The bulk of my effort for TS right now is going to focus on moving those versions forward towards stable and hopefully getting onto the latest TS version for alpha/beta versions so we can leverage TS's support for decorator metadata.

As it pertains to the archive branches - we can try to get some support there, but I would suggest starting to plan on how to move off those. Given the alpha/beta changes have been moving slower, we've continued to backport some fixes, but I don't see that going on forever. At some point, the code there is just difficult to maintain with backwards compatibility, especially given the amount of the time the FAST Components package has been deprecated. I'm keeping this open with the hopes we can get to updating the archive, but changes there are more difficult and time consuming as of now.

Good news - I repro'd this locally when trying to update the repo. Let's see if @nicholasrice has any concerns with this; he may have ideas on where/why the constraint may be failing in this scenario as well.

[chrisdholt]

Update on this - I got some insight from the TS team on why this is throwing now. I think we need to look at the best path forward and update this w/ TS.

[chrisdholt]

Made it through getting the repo building with TS 4.8+, unfortunately it requires changing out all the test suites due to ESM requirements. Building this locally and if it passes I plan to push it through. Thanks @fcollonval for this PR and all for your patience.

[fcollonval]

Thanks @chrisdholt for taking the time of looking at this maintenance PR.

[rajsite]

Made it through getting the repo building with TS 4.8+, unfortunately it requires changing out all the test suites due to ESM requirements. Building this locally and if it passes I plan to push it through. Thanks @fcollonval for this PR and all for your patience.

That's awesome news, thanks @chrisdholt! Do you know when we might see the changes in a published package?

[rajsite]

Made it through getting the repo building with TS 4.8+, unfortunately it requires changing out all the test suites due to ESM requirements. Building this locally and if it passes I plan to push it through. Thanks @fcollonval for this PR and all for your patience.

That's awesome news, thanks @chrisdholt! Do you know when we might see the changes in a published package?

These changes are now in @microsoft/fast-foundation@2.49.2 🎉