writing string null value

[Springwald]

Hi,

when I try the following (saving a string value to the storage):

private ILocalStorageService localStorage; .... await localStorage.SetItemAsync(key, null); var value = await localStorage.GetItemAsync(key);

the content of value is not null but a string with the content 'null' (the word null without question marks)...

Best Daniel

[Springwald]

I use this wrapper method as a temporary, working bugfix:

public async Task<T> GetValue<T>(string key)
{
    var value = await localStorage.GetItemAsync<T>(key);

    // >>> bugfix for Blazored.LocalStorage which returns "null" instead of null 
    var type = typeof(T).Name;
    if (type  == "String")
    {
        if (value == null) return default(T);
        string v = value.ToString();
        if (v == "null") return default(T);
    }
    // <<<

    return value;
}

But this way, of course, I cannot distinguish whether the word zero or the value zero has been stored...

[chrissainty]

Hi @Springwald, could I ask why you need to save null? What value is there is saving nothing?

[Springwald]

Hello,

I'm afraid I don't understand the question.

I want to save and load the type "String" without changing it. After saving and loading again, the content should be unchanged. This is what any normal memory and serialization does.

A use case is for example:

I store a User-ID as string. The first time the user is not logged in, so the string is null. Then he logs in, and I store an ID. When he logs out again, I set the string back to null. But here it contains "null" as a string...

Many greetings Daniel

[chrissainty]

In your example you are not saving a string, you are saving null (nothing). Do you mean you want to save a key to local storage without a value?

[Springwald]

I just want to save a string. And the default value of string is null and not "null".

maybe in this case:

string loginId = await localStorage.GetItemAsync<string>("loginid");
bool isLoggedIn = loginID != null;

but here I have to check

bool isLoggedIn = loginId != "null";

what is very unexpected

[chrissainty]

So just to be clear, the behaviour might seem unexpected but it's actually as per the spec. There is no way to save a null into local storage. Could you not save an empty string instead?

[Springwald]

An empty string and the default value null of string are not identical. In this example, this might work, but in many other places this represents a dangerous loss of information. Furthermore, the linked specification is for Javascript, where there is the state null but also the state undefined...

If this can't be solved, you might want to avoid saving simple data types and only allow classes to be saved.

[chrissainty]

This library calls into JavaScript APIs hence we're at the mercy of that behaviour. As it shows in the spec I linked, it's impossible to save null to local storage as the setItem function only accepts a DOMString. In JavaScript, a null value will be stringified to "null", which is the behaviour you're seeing.

I agree an empty string and null are not the same but that wasn't something I was suggesting. I was suggesting that instead of saving null and checking for null you save an empty string and check for an empty string. Personally, if you're not saving a value I wouldn't save anything at all and just check for the presence of the key, if it exists there is something to read and if not you have your null scenario.

[Springwald]

But that would be very risky in my eyes.

You should never forget to write something like this every time you come here:

await localStorage.SetItemAsync<string>(key, value ?? String.Empty);

and while reading you must not forget

var value = await localStorage.GetItemAsync<string>(key);
if (String.IsNullOrEmpty(value)) value = null;

Because in C#, I can't make sure a string isn't null before...

As I said, if that couldn't be solved, you might be able to force a wrapper class for simple data types. So that nobody like me falls for it.

Then I could only send it like this...

var loginStatus = new LoginStatus {
        LoginId = null
    }
await localStorage.SetItemAsync<string>(key, loginStatus);

[chrissainty]

I've taken this approach with several projects and not had an issue, but I appreciate I understand the behaviour of the JavaScript APIs. If I'm writing a value to local storage, for example saving an access token. I'd check if it was null first, if it was I wouldn't write anything. That way when I check for the key later, I know that if it's present I can be confident there is a value there.

So to save us going round in circles on this subject now we've established the behaviour is inline with the underpinning API. What do you want to see happen?

[Springwald]

At this point I have no more expectations ;-) I thought it was just a bug and not a philosophy ;-)

But the problem is not that unlikely: For me it was the first contact with your localstorage and it took me 1 hour to find out the reason.

From my point of view your localstorage is a C# service. And Blazor is just for the dotnet developers who don't want to build with Javascript like Angular or VueJs. So I wouldn't assume that the typical C# developer, when using a C# service, would not know that he has to take care of things that have to do with Javascript at the very bottom.

So I can only tell you what I personally would do: When I create access to localstorage for the C# world, I would try to make strings behave like in the C# world, not the Javascript world. This would mean, (for example, in unit tests) making sure that all string variants of the C# world come in and out unchanged. Or you could check that no one enters the type "string" and then throw an exception that your storage cannot do that.

[chrissainty]

It's not a philosophy, not sure what you mean by that.

This is the first time in 2 years that this has come up so I don't think this has been a major sticking point for most people. However, as Blazor gets more popular I appreciate more C# developers, who perhaps don't understand the web that well, are going to be using this library and might hit this issue.

I'm going to leave this issue open and have a think about the options. I think the most likely route is to throw if anyone tried to save a null value as that seems the most reliable method.

[Springwald]

That sounds like a good plan to me. Thank you for taking the time to clarify the circumstances for the first step.

[intech-paul]

Can I add something ? This sounds like a discussion that the old win32 api GetProfileString/GetPrivateProfileString does, you provide a default value in the call that in case the value does not exist in storage, it returns that value.

[Assassinbeast]

I agree 120% with Springwald on this.

If you set something to null, and retrieve it, then you expect null back, and not the string "null".

Its the same if you used for example used a C# Dictionary<string, string> dic.

If you set dic["name"] = null, then dic["name"] gives you a null, not "null" string value

I just spend over an hour trying to find the bug in my app... because i set something to null, and then when i retrieved it, i checked the value with string.IsNullOrWhiteSpace(...), and then it was false, when in fact it should be true.

I would recommend that if you set something to null, then you should delete the key/value from LocalStorage.

[r-Larch]

Hi everyone,

I just want you to know, that all these edge cases get well handled by JsonSerializer. So just use it to write any string, either null or normal string, which results in the correct behavior. Only side-effect: strings get serialized with quotes, but I wouldn't consider this as bad, because it's the same behavior as JSON.stringify(...) and JSON.parse(...).

Behavior with System.Text.Json:

// using System.Text.Json;
// using System.Text.Json.Serialization;

{
    // writing null as json:
    var value = JsonSerializer.Serialize((string) null);
    Debug.Assert(value == "null");
}
{
    // reading null from json:
    var value = JsonSerializer.Deserialize<string>("null");
    Debug.Assert(value == null);
}

{
    // writing a string as json:
    var value = JsonSerializer.Serialize("Some String");
    Debug.Assert(value == "\"Some String\"");

}
{
    // reading a string from json:
    var value = JsonSerializer.Deserialize<string>("\"Some string\"");
    Debug.Assert(value == "Some string");
}

Behavior with javascript:

{
    // writing null as json:
    var value = JSON.stringify(null);
    console.assert(value == "null");
}
{
    // reading null from json:
    var value = JSON.parse("null");
    console.assert(value == null);
}

{
    // writing a string as json:
    var value = JSON.stringify("Some String");
    console.assert(value == "\"Some String\"");

}
{
    // reading a string from json:
    var value = JSON.parse("\"Some string\"");
    console.assert(value == "Some string");
}

I would Blazored.LocalStorage let behave like this:

var original = null;
localStorage.setItem('test', JSON.stringify(original))
var result = JSON.parse(localStorage.getItem('test'));
console.assert(result == original)

var original = 'Some string';
localStorage.setItem('test', JSON.stringify(original))
var result = JSON.parse(localStorage.getItem('test'));
console.assert(result == original)

[chrissainty]

Thanks for the information @r-Larch. We use System.Text.Json to handler serialisation and de-serialisation so I'm wondering why people are seeing these issues?

[r-Larch]

The behavior can be reproduced with:

var key = "test";
await LocalStorage.SetItemAsync<string>(key, null);
var value = await LocalStorage.GetItemAsync<string>(key);
Debug.Assert(value == "null");

What's going on is: The method SetItemAsync<T>(..) skips JSON serialization for all strings, so strings get stored as "plain strings" (strings without quotes), and because of (null is string) == false null gets serialized and ends up as "plain string" as well, so later the GetItemAsync<T>(..) method can no longer distinguish between the "plain string" null and a real string value so it returns null as a string.

To address the issue, I would change the following:

https://github.com/Blazored/LocalStorage/blob/9d724b0e708c3897c376c8e2bd4c7954c51a4433/src/Blazored.LocalStorage/LocalStorageService.cs#L22-L67

To something like:

public async ValueTask SetItemAsync<T>(string key, T data)
{
    if (string.IsNullOrEmpty(key))
        throw new ArgumentNullException(nameof(key));

    var e = await RaiseOnChangingAsync(key, data).ConfigureAwait(false);

    if (e.Cancel)
        return;

    // serialize everything:
    // the serializer takes care of all edge cases
    // - null          => 'null'
    // - "some string" => '"some string"'
    // - empty string  => '""'
    // - some object   => '{"prop": 123}'
    // - some array    => '[1,2,3]'
    var serialisedData = JsonSerializer.Serialize(data, _jsonOptions);
    await _jSRuntime.InvokeVoidAsync("localStorage.setItem", key, serialisedData).ConfigureAwait(false);

    RaiseOnChanged(key, e.OldValue, data);
}

public async ValueTask<T> GetItemAsync<T>(string key)
{
    if (string.IsNullOrEmpty(key))
        throw new ArgumentNullException(nameof(key));

    var serialisedData = await _jSRuntime.InvokeAsync<string>("localStorage.getItem", key).ConfigureAwait(false);

    if (string.IsNullOrWhiteSpace(serialisedData))
        return default;

    // deserialize everything:
    // the serializer takes care of all edge cases 
    // and a {System.Text.Json.JsonException} gets thrown 
    // if the json string can not be converted to the generic type {T}
    // - 'null'          => null
    // - '"some string"' => "some string"
    // - '""'            => empty string
    // - '{"prop": 123}' => some object
    // - '[1,2,3]'       => some array
    return JsonSerializer.Deserialize<T>(serialisedData, _jsonOptions);
}

[r-Larch]

For backward compatibility, you could catch the JsonException and return a plain string. So people could update the library without breaking changes.

...
public async ValueTask<T> GetItemAsync<T>(string key)
{
    if (string.IsNullOrEmpty(key))
        throw new ArgumentNullException(nameof(key));

    var serialisedData = await _jSRuntime.InvokeAsync<string>("localStorage.getItem", key).ConfigureAwait(false);

    if (string.IsNullOrWhiteSpace(serialisedData))
        return default;

    try {
        // deserialize everything:
        // the serializer takes care of all edge cases 
        // and a {System.Text.Json.JsonException} gets thrown 
        // if the json string can not be converted to the generic type {T}
        // - 'null'          => null
        // - '"some string"' => "some string"
        // - '""'            => empty string
        // - '{"prop": 123}' => some object
        // - '[1,2,3]'       => some array
        return JsonSerializer.Deserialize<T>(serialisedData, _jsonOptions);
    }
    catch (JsonException e) when (
        // the error happened on the first token, so it could be some alphanumeric character:
        e.Path == "$" &&
        // and the caller expects a string:
        typeof(T) == typeof(string)
    ) {
        // For backward compatibility return the plain string.
        // On the next save a correct value will be stored and this Exception will not happen again, for this 'key'
        return (T) (object) serialisedData;
    }
}

[chrissainty]

This is a great suggestion @r-Larch and makes total sense. I've tracked down why we originally put this in place (https://github.com/Blazored/LocalStorage/pull/59/files?file-filters%5B%5D=.cs#diff-94f56c8c1f77cafd71ae61e27fa9977d49a35c067b792408e29ca55db92fd67d). But in hindsight, I don't think it was the right move.