strawberry
strawberry copied to clipboard
default_factory doesn't work
Hi! I use default_factory to initialize my variable, but variable always returns the same result. It seems like default_factory doesn't work and it returns always the same result of function.
Here is example to reproduce: https://play.strawberry.rocks/?gist=a7a5e62ffe4e68696b44456398d11104
I can reproduce this, I think it's because we store the default value when creating the object, see:
Fixing this will potentially be a breaking change 🤔
@coady what do you think of this issue?
I don't think this can be supported (and still be compliant with the spec). GraphQL defaults are statically in the schema. strawberry export-schema ...:
type Mutation {
update1(fields: MyInputType!): String!
update2(fields: MyInputType!): String!
}
input MyInputType {
field1: String = "318fbf6e-73b6-40eb-932f-0b66ba935b75"
}
type Query {
hello: String!
}
Another issue is that the client sending an explicit null is valid and semantically different. So MyInputType would need logic like
if field1 in (None, UNSET):
field1 = uuid_pkg.uuid4()
That may seem like a workaround, but is actually the only correct implementation.
@patrick91 @coady hi!
Thanks for your answers. Yes, now it's the only way with None, UNSET.
The main reason why I'm thinking about another implementation is pydantic models, I really like to use @strawberry.experimental.pydantic.input and when I inherit a model with default_factory, then this default_factory is inherited in the field too and I can't override this field with UNSET and None
example:
import strawberry
from pydantic import BaseModel, field
class PydanticModel(BaseModel):
field1: str | None = Field(default_factory=uuid_pkg.uuid4)
@strawberry.experimental.pydantic.input(PydanticModel)
class MyInputType():
field1: str | None = strawberry.UNSET # not works here
By the way I sent daft PR with overriding https://github.com/strawberry-graphql/strawberry/pull/3469
@patrick91 and also can I ask in what situations then we need to use default_factory if this field is static, when we can use only default?
@patrick91 and also can I ask in what situations then we need to use
default_factoryif this field is static, when we can use onlydefault?
I'm not sure to be honest, I'll need to think about this a bit
I do think it might be a flaw, or at least something surprising, so maybe we need to reconsider it
I do understand the schema's default value issue, but I do agree with this comment. I actually had to do workaround a similar issue on strawberry-resources when exporting form data for the field as a dynamic default value would not actually make sense there.
So my vote would be to actually change the behavior to fix this issue, specially since we still are 0.x =P, and mention as a "possible breaking change" in the changelog, mentioning the use of default as the correct way of relying on the older behavior
just throwing out some ideas to make the change less painful
- we could a
static_factoryorschema_factorywhich will have the current behaviour,default_factorywill mimic dataclasses' and pydantic's behaviour - have a configuration option to disable static defaults
- just change the behaviour
option 1 could also have a codemod to make the update easier 😊
@coady sorry to ping you again, but do you use default_factory for defaults in the schema level? 😊 what's your use case exactly?
@coady sorry to ping you again, but do you use default_factory for defaults in the schema level? 😊 what's your use case exactly?
The only use I'm aware of (and use) is for mutables, as dataclasses requires. Any valid value is a valid default value, including [] and {}.
I'd be happy with a cleaner alternative for mutables. This is forbidden (but is valid GraphQL):
q: list[float] = [0.5]
So instead I have to use default_factory or this:
q: list[float] = (0.5,) # type: ignore
which mypy complains about.
just throwing out some ideas to make the change less painful
- we could a
static_factoryorschema_factorywhich will have the current behaviour,default_factorywill mimic dataclasses' and pydantic's behaviour- have a configuration option to disable static defaults
- just change the behaviour
option 1 could also have a codemod to make the update easier 😊
I think we could do option 2 for the time being