Skip to content
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

[Feature Request] Support Strongly Typed Serialization #211

Open
SGStino opened this issue Jun 2, 2023 · 6 comments · May be fixed by #241
Open

[Feature Request] Support Strongly Typed Serialization #211

SGStino opened this issue Jun 2, 2023 · 6 comments · May be fixed by #241
Labels
Feature Request Request to add a new feature Needs: More Information The author needs to provide more information on the issue.

Comments

@SGStino
Copy link

SGStino commented Jun 2, 2023

The current serialization model assumes JsonSerializer.Deserialize() will always work: (here)

    private async Task<ChangingEventArgs> RaiseOnChangingAsync(string key, object data)
    {
        var e = new ChangingEventArgs
        {
            Key = key,
            OldValue = await GetItemInternalAsync<object>(key).ConfigureAwait(false),
            NewValue = data
        };

        Changing?.Invoke(this, e);

        return e;
    }

This means that as soon as you use a JsonTypeInfoResolver that doesn't know what to do with object, like a source generated JsonSerializerContext, you essentially can't use the library anymore.

I'm not certain one could get away of making RaiseOnChangingAsync Generic, in most cases where the old value and the new value are of the same generic type, AND haven't changed over time, that would work.

But if one were to put different types in the same key, then that would certainly break. Where the old value is of OldType, the new of NewType: that would invoke RaiseOnChangingAsync<NewType>, which on its turn would invoke GetItemInternalAsync<NewType> instead of GetItemInternalAsync<OldType>, and you'd have a conflicting deserialization.

One could skip the entire ILocalStorageService and use IStorageProvider directly with custom, fixed, serialization, but the accessibility of internal interface IStorageProvider sadly doesn't allow that.

The alternative that remains is to use GetItemAsStringAsync, without a JsonSerializationContext on the library, and use a Serialization Context to generate the string. Which seems to work, but with a fair amount of overhead.

@SGStino SGStino added Feature Request Request to add a new feature Triage Issue needs to be triaged labels Jun 2, 2023
@chrissainty
Copy link
Member

Hi @SGStino, could you provide a sample project showing the issue you're describing. I'm struggling to understand the issue based on the description.

@chrissainty chrissainty added Needs: More Information The author needs to provide more information on the issue. and removed Triage Issue needs to be triaged labels Jun 6, 2023
@SGStino
Copy link
Author

SGStino commented Jun 9, 2023

When using trimming and serialization, you can't deserialize , you need to specify the type, and the type needs to be known in the source generated serialization context.

@chrissainty
Copy link
Member

I'll be honest, I'm not sure I understand what you're attempting to do. I haven't done any work whatsoever with source generators.

I think, for now at least, this is something you would need to handle in your own code and just use the GetItemAsStringAsync and SetItemAsStringAsync methods. I'd be happy to look at this again if there is a big demand for it.

@SGStino
Copy link
Author

SGStino commented Nov 21, 2023

Well, the AsString functions are exactly what i'm using now in a custom service that wraps it and uses a DI injected context for the serializer.

Ps: if you want to know more about the source generation in System.Text.Json:
https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/source-generation?pivots=dotnet-8-0

@MatthewSteeples
Copy link

I think this is now supported thanks to some new APIs added to .NET 7/8. I've put the following in my program.cs file:

builder.Services.AddBlazoredLocalStorage(options => { options.JsonSerializerOptions.TypeInfoResolver = SerializationContext.Default; });

SerializationContext is what I've called my source generated JSON class

@MatthewSteeples
Copy link

OK, so I was a bit premature there. There's a small code path that isn't happy about enabling source generators. I'm going to see if I can put together a PR to fix it

MatthewSteeples added a commit to MatthewSteeples/LocalStorage that referenced this issue Mar 11, 2024
@MatthewSteeples MatthewSteeples linked a pull request Mar 11, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Request to add a new feature Needs: More Information The author needs to provide more information on the issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants