Some types should be json, not just `any` or `{}`

[leaumar]

I'd like to suggest adding a json type in some places, to have more correct/helpful types to work with.

Two places that immediately come to mind are browser.runtime.onMessage and browser.storage.<area>.get. Both of these can only ever emit json objects (the latter even with predictable keys), so it makes sense to type them as such to make consuming code simpler and more correct. The former currently uses any, which disables all type checks, and the latter uses {}, which makes field access clunky on top of widening the value types.

type JsonPrimitive = null | boolean | number | string;
type JsonObject = {
    [key: string]: JsonValue | undefined;
};
type JsonArray = JsonValue[];
type JsonValue = JsonPrimitive | JsonArray | JsonObject;

These types would prevent expecting non-json values like Date, and would allow access of any field on JsonObjects but still force type checks on the values.

~If you're not opposed to this for some reason, I could try to write up a PR.~ I just had a look at how this library is generated, I was expecting to be able to write typescript types, not openapi-style flat objects...

[Lusito]

Hi @leaumar, thanks for the suggestion. My thoughts about this:

I've tried using this type of JSON type in the past in other projects and there have always been some cases, where a type would not be accepted even though it fulfilled the definition of the JSON type in theory.

While I do like the idea of having more safe types, I am worried about the wrong errors that will be caused by this.

I had most of these issues at work.. I'll check if I can reproduce the issues with current typescript.

Maybe this has gotten better. After all, typescript is progressing fast.

About the code-generation: I don't like the schema format either, but since that's how mozilla maintains them, it's out of my hands. The only other solution would have been to manually write the types and having to maintain them manually as well. At least this way I can maintain the library with little effort.

In this case, the proper way to create a PR would be to use fixes.json to apply changes to the schemas before code-generation. This of course requires some knowledge about the schema.json files and some info about how the fixes.json works.

Not very beginner-friendly I'm afraid.

[SantoJambit]

So, here's one case that should work, but doesn't:

interface MyType {
    foo: string;
}
const foobar: MyType = { foo: 'bar' };
function test(fb: JsonValue) {
    return 0;
}
/*
  Error: Argument of type 'MyType' is not assignable to parameter of type 'JsonValue'.
  Type 'MyType' is not assignable to type 'JsonObject'.
    Index signature is missing in type 'MyType'
*/
test(foobar);

Interestingly, when you use this it works:

type MyType = {
    foo: string;
};

But the problem remains: Even though you would think this is a JSON type, it doesn't match and you'd have to use "as any".

Feel free to discuss this further.

(this is my work account)

[leaumar]

Yeah, I've been noticing it too when POCing this in my own code, the index signature means you can't make concrete instances of the type. I'm not sure how to get around that. The higher level UX issue remains valid IMO, that the types should match as closely as possible to reality, but I agree the implementation is not easy and there's no concrete idea for the fix yet.

[Lusito]

Well, I'll leave this open as a feature request and once someone comes up with a good solution, I'll be happy to add it.