Uptake fast-foundation fixes and remove workaround

[m-akinc]

Pull Request

🤨 Rationale

Fixes #1661

👩‍💻 Implementation

• Updated fast-foundation to 2.49.5
• Removed bug workaround from anchor menu item

🧪 Testing

Verified issues fixed in Storybook.

✅ Checklist

• I have updated the project documentation to reflect my changes or determined no changes are needed.

[rajsite]

The PR is still failing builds and looks like it was prematurely set to owners review, moving back to draft

[m-akinc]

I discovered that this new release of fast-foundation introduces an issue on Chrome/Edge. If a component's styling uses a themeBehavior, switching to the theme crashes the window (i.e. by setting the theme value of the containing theme provider). There seem to be exceptions, such as the menu and the select (both of which's themeBehaviors style only slot elements).

We can't update to this release of fast-foundation with this issue. If possible, we should try to fix this in FAST, but reporting a Chromium bug may also be an option.

[rajsite]

There is only one other change in fast foundation that was not contributed by us: https://github.com/microsoft/fast/commits/archives/fast-element-1/

Are you saying we introduced the regression? If so how did we test the change we proposed to FAST. We may need to make sure we are doing an npm pack and installing the built tgz to test against our own code.

[rajsite]

There was one other release before the one with our changes: microsoft/fast@7148131

Do you see the same failures on just that change?

Edit: validated below that the release before the one with our changes did not cause issues and seems to be in wide use already

[rajsite]

We can't update to this release of fast-foundation with this issue

Then we may need to quickly release a version of nimble that pins our FAST version. It was a patch release so any app that does a general dependency update will start using the new version of FAST.

If we introduced the regression and it is observable in FAST we probably want to notify them and get them to deprecate the package and warn their users not to upgrade.

If it is a major issue there are 72 hours to remove the package, after that it can only be deprecated: https://docs.npmjs.com/unpublishing-packages-from-the-registry

Guessing folks have some automated tooling for pulling it in, already has several hundred downloads

@jattasNI

[rajsite]

Searching all ASW most apps are on 2.49.4 and some are older. Seems unlikely that 2.49.4 introduced the regression.

I was able to reproduce the crash in the storybook angular example app by just changing the theme to dark.

I'm going to force through a PR that locks us on fast-foundation 2.49.4: #1779 (update: merged) (update: released as nimble-components 21.0.4)

[rajsite]

Gave the fast folks a heads-up on discord: https://discord.com/channels/449251231706251264/1197942296931483669/1199912333787541575

Reproduced as follows:

rajsite — Today at 9:03 PM
Chris Holt heads-up that during integration of [email protected] we found a regression that causes a crash in chrome (121.0.6167.86) and edge (120.0.2210.144) but not firefox (122.0) or safari (ipados 16.7.5).
We haven't narrowed down the cause to see if its a chrome/edge bug, a problem in the implementation we contributed to FAST, or an issue manifesting in our token usage (which is quite complex as we leverage tokens for many things). While investigating we are pinning users on [email protected].
You can see an example of the crash in chrome or edge in this example with 2.49.5 by going to the top right menu > user settings > and changing the select to dark mode: https://60e89457a987cf003efc0a5b-gnaiujxbdz.chromatic.com/example-client-app/#/customapp
And that the crash is not reproducible in any of the above browser with the 2.49.4 build: https://60e89457a987cf003efc0a5b-affprjyukn.chromatic.com/example-client-app/#/customapp
Chris Holt — Today at 9:03 PM
This is the latest version, yes?
rajsite — Today at 9:04 PM
Yep
Chris Holt — Today at 9:05 PM
Ok, I likely can’t test tonight, but my first approach is probably to check against other versions to isolate whether it’s related to the change or related to the browser. Then I’d probably go from there. If folks on your end can isolate, we can go from there - or I’ll look as soon as I’m able
rajsite — Today at 9:07 PM
Sounds good, we likely won't be able to investigate further tonight but will keep looking tomorrow

[rajsite]

Pinned the fast-foundation version so unlikely that our users that update dependencies will pick-up the version with the regression and notified the fast folks on discord.

The next steps are to investigate the issue further and see if we need a patch / revert the change in fast (ideally ASAP) and if we need to report an issue to the browser vendors (but either way we should avoid leaving fast-foundation in a state breaking current browsers)

[m-akinc]

We're talking to Chris Holt in a Discord thread about possible workarounds we could submit to FAST. I also filed a bug to Chromium.

[rajsite]

I also had to pin @microsoft/fast-react-wrapper as the latest version tried to bring in 2.49.5 as well in #1771 . This all may be less painful if fast switches to peerDependencies, I'm not actually sure (and it's a change in main they are proposing so who know when it will release): microsoft/fast#6908

Eitherway as part of resolving the upgrade we should unpin fast-foundation and fast-react-wrapper