-
-
Notifications
You must be signed in to change notification settings - Fork 4
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
How to interpret simple schemas that contains additionalProperties
#12
Comments
additionalProperties
additionalProperties
My view is that from the type system's point of view type Foo = { a: string };
function doSomething(n: Foo) {
console.log(foo.a.endsWith("a")); // foo.a is a string
}
const aaa: Foo = { a: "a" };
doSomething(aaa); // No type error
const bbb = { a: "a", b: "b"};
doSomething(bbb); // No type error "bbb" is a Foo with additional property "b" and "doSomething" will happily except it. The type system will complain if you try to use it, but there's no problem with it being there. The equivalent of Foo with I think it's reasonable for this to extend to So, for type generation, my suggestion would be to ignore If people want to expose additional properties as a property with an object map, this is where an idl-vocab keyword could be useful to allow people to specify that that's what they want and the name of the property where they can access the object map of additional properties. |
Hmm, that is definitely another way to look at the keyword 🧐 It is only because TS assume const bbb: Foo = { a: 'a', b: 'b'}; // Type error I see One of the use-cases for {"name": "root", "properties": {"accounts": { "additionalProperties": {... Account info object } } } } which enables data such as: { "accounts": { "account-1": { ... } } } Where you really want to have access to the There is also a huge difference between when type Root = {
accounts: { [key: string]: Account }
}; Otherwise, if {"name": "root", "properties": { "accounts": { "name": "Accounts", "properties": { "prop": {...} }, "additionalProperties": {... Account info object } } } } It becomes difficult to render the map of accounts, without specifying a property that contains the accounts in some way such as: type Root = { accounts: Accounts };
type Accounts = { prop: ..., additionalProperties: { } }; If one would not want the object to contain extra properties why would one specify anything other than But I agree, we need a keyword to control this behavior regardless. Any suggestions? 😅 |
Hey, since @jonaslagoni asked me to provide some feedback, here some general thoughts how we handle this case at TypeSchema, which might be also useful for your case. In our logic, if a schema contains an {
"type": "object",
"additionalProperties": {
"type": "string"
}
} In Java we try to convert this schema to {
"type": "object",
"additionalProperties": {
"$ref": "My_Type"
}
} In this case we would generate a Java type like So as general suggestion, if we try to build a schema for code generators I can only recommend to make some hard restrictions on the flexible nature of JsonSchema, otherwise you end up with cases which are too complicated to handle for code generators and which also limits the target languages which can implement such solutions so "keep it simple stupid". In the end we have also chosen this path for TypeSchema. |
No, it's because that's how polymorphism works. It works like this in every OO language. I could have defined a type that extends Foo and used that instead of an type Foo = { a: string };
type Bar = Foo & { b: string };
function doSomething(n: Foo) {
console.log(foo.a.endsWith("a")); // foo.a is a string
}
const aaa: Foo = { a: "a" };
doSomething(aaa); // No type error
const bbb: Bar = { a: "a", b: "b"};
doSomething(bbb); // No type error
class Foo {
String a;
public Foo(String a) {
this.a = a;
}
}
class Bar extends Foo {
String b;
public Bar(String a, String b) {
super(a);
this.b = b;
}
}
class Main {
public static void main(String[] args) {
Foo aaa = new Foo("a");
doSomething(aaa); // No type error
Bar bbb = new Bar("a", "b");
doSomething(bbb); // No type error
}
private static void doSomething(Foo foo) {
System.out.println(foo.a.endsWith("a")); // foo.a is a string
} When you do something like
I see this as an abuse of
It's difficult to express a type for this because it's not a usable data structure. Imagine what code that works with this type would look like. To get at the accounts, you would have to iterate over the whole object while ignoring known properties. It's not our responsibility to change their data structure to make it make sense by adding an artificial property. They should get what they ask for. |
@chriskapp Welcome! Glad to have you in the conversation! Essentially, you side-step the problem we're trying to solve here by not allowing |
I'm on the fence between "ignore it" and "create a map-type property." But I don't think that the inheritance model holds up, except that It was touched on before, but I'll reiterate. If we have I think the best we could do for the polymorphic case is to support |
I agree that |
We are gonna focus on the very basic schemas which has
additionalProperties
defined.How do we want to interpret
additionalProperties
?Defining additionalProperties allow people to provide additional properties beyond what
properties
define.This means that the following schema:
Validates correctly against:
And we need to figure out how we want to give access to
key2
, if we want that of course.I see two options
From a data definition standpoint, I see two options for how we can interpret it.
additionalProperties
property for the "class", which uses the appropriate dictionary with a key of type string and value of type, whateveradditionalProperties
in the schema resolve.Side effects of options 2
If we choose option 2, we have a couple of extra things we need to discuss:
additionalProperties
we need to decide how we want to handle such duplication of properties.additionalProperties
property, which can cause clashes of properties, say I have the very same property defined underadditionalProperties
as one of the core properties. Should we simply ignore such properties?The text was updated successfully, but these errors were encountered: