Add try catch in FluentDesignTheme

[MarvinKlein1508]

Pull Request

📖 Description

This PR adds a try-catch block to FluentDesignTheme.razor.js to prevent site crashing on invalid data within the localStorage. This makes it easier to migrate to FluentUI in existing projects which uses the same localStorage keys to store theme information.

For example execute this in the browsers console:

localStorage.setItem("theme", "light");

You'll notice that the website will crash until you manually clear the localStorage.

I had this issue already when switching from certain Bootstrap Frameworks to FluentUI. This PR prevents this crashes and will automatically clear the localStorage for the theme when JSON.parse fails

👩‍💻 Reviewer Notes

You can get around with it by providing the FluentDesignTheme another key. But in my opinion it should check use try-catch anyways in case the object will get changed in the future.

📑 Test Plan

✅ Checklist

General

• 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.

Component-specific

• I have added a new component
• I have added Unit Tests for my new compontent
• I have modified an existing component
• I have validated the Unit Tests for an existing component

⏭ Next Steps

[vnbaaij]

Good addition, I think.

[dvoituron]

To maintain good performance, it's never a good idea to add a Try/Catch.
But in this case, it's probably negligible.

On the other hand, I don't see how the LocalStorage could contain a value not supported by the library itself. Unless the dev or the user manually modifies this value... and in that case, too bad for him :-)

[MarvinKlein1508]

@dvoituron this can happen when you migrate from another library to FluentUI. In My case I've migrated from a Bootstrap Libary to FluentUI. This library also uses localStorage to store theme details in key theme.

Every old user visiting the site will get an exception now until they clear their localStorage.

[dvoituron]

Yep. It's not a FluentUI Blazor issue 😀 You could add a script to clean your user environments.

[MarvinKlein1508]

@dvoituron yes. Right now I'm doing this as a workaround:

public class ClearSiteDataMiddleware
{
    private readonly RequestDelegate _next;

    public ClearSiteDataMiddleware(RequestDelegate next)
    {
        _next = next;
    }

    public async Task InvokeAsync(HttpContext context)
    {
        // Add the Clear-Site-Data header to the response
        context.Response.OnStarting(() =>
        {
            context.Response.Headers.Append("Clear-Site-Data", "\"cache\", \"storage\"");
            return Task.CompletedTask;
        });

        // Call the next middleware in the pipeline
        await _next(context);
    }
}

but how would I know when I actually cleaned the cache/localStorage for every user? As long as this script is active, the user cannot switch the theme in the app.

[dvoituron]

You could run a JS Script immediately in your "index.html" page (detecting if this localstorage is a Json or not)

[MarvinKlein1508]

@dvoituron I don't have an index.html only App.razor. Also doesn't get the JS from the component get called before my script will get the chance to execute?

[dvoituron]

@dvoituron I don't have an index.html only App.razor. Also doesn't get the JS from the component get called before my script will get the chance to execute?

You must have a "first HTML" page: where the "Blazor.js" file is defined. All Blazor components are initialized after these JS files.

[dvoituron]

But we can merge your PR. It's more about the technical reason for this request.