[codegen models] add 'required' modifier in constructors when a field is non-nullable

[dJani97]

Description

The models generated by amplify codegen models won't compile because of the following error:

The parameter 'id' can't have a value of 'null' because of its type, but the implicit default value is 'null'. (Documentation)
Try adding either an explicit non-'null' default value or the 'required' modifier.

The generated field definition looks like this:

final String id;

And the generated constructor looks like this:

const Someclass._internal({this.id});

Since this field is non-nullable, the constructor should be generated like so:

const Someclass._internal({required this.id});

This causes us some overhead as we have to update the generated files manually each time.

We're on CLI version 12.0.3 and Flutter version 3.10.2.

Categories

• [ ] Analytics
• [ ] API (REST)
• [ ] API (GraphQL)
• [ ] Auth
• [ ] Authenticator
• [ ] DataStore
• [ ] Storage

Steps to Reproduce

Have a model with a non-nullable field.

Run amplify codegen models.

Try compiling they project with the latest version of Flutter.

Screenshots

No response

Platforms

• [ ] iOS
• [ ] Android
• [ ] Web
• [ ] macOS
• [ ] Windows
• [ ] Linux

Flutter Version

3.10.2

Amplify Flutter Version

1.1.0

Deployment Method

Amplify CLI

Schema

No response

[Equartey]

Hey @dJani97, sorry you're having this issue.

Codegen should indeed mark that field as required.

I tried generating a simple model using the same versions as you and codegen marked it required as expected.

Can you provide the schema you're working with?

[HuiSF]

Hi @dJani97 The codegen generated code may unnecessarily conform all linter rules as the comment inserted in the generated file:

// NOTE: This file is generated and may not follow lint rules defined in your app
// Generated files can be excluded from analysis in analysis_options.yaml
// For more info, see: https://dart.dev/guides/language/analysis-options#excluding-code-from-analysis

We recommend you to do so to avoid linter noise.

[Equartey]

Hey @dJani97, are you still facing this issue?

If so can you please provide your schema? And are you using any feature flags with Amplify CLI?

[dJani97]

@HuiSF These are not lint warnings, but compilation errors, so I can't just ignore them:

Error: The parameter 'id' can't have a value of 'null' because of its type 'String', but the implicit default value is 'null'. Try adding either an explicit non-'null' default value or the 'required' modifier.

---

@Equartey For example this schema:

type SomeModel {
  id: String
  name: String
}

generates the following model:

@immutable
class SomeModel {
  final String id;
  final String? _name;

  String? get name {
    return _name;
  }
  
  const SomeModel._internal({this.id, name}): _name = name;
  // ...
}

Even though both fields are using the String notation instead of String!, for some reason, Amplify knows that the id: String field should be non-nullable. And so it makes it non-nullable in the field definition, but it doesn't apply the required keyword to the constructor:

Now I understand that this could be fixed by updating the model like so:

type SomeModel {
  id: String!
  name: String
}

... but still, this behavior is confusing and if amplify will make all id fields required by default, then it should also make it required in the constructor.

---

Edit: I consulted with backend, and they say that this field IS actually nullable on their side. So this is why they added it to the schema this way. Why does Amplify CLI make this decision for us, and treat all id fields as non-nullable by default?

[Jordan-Nelson]

@dJani97 - I am not able to reproduce this.

The following schema:

type SomeModel @model {
  id: String
  name: String
}

produces the following generated code:

class SomeModel extends Model {
  static const classType = const _SomeModelModelType();
  final String id;
  final String? _name;

  @override
  getInstanceType() => classType;
  
  @Deprecated('[getId] is being deprecated in favor of custom primary key feature. Use getter [modelIdentifier] to get model identifier.')
  @override
  String getId() => id;
  
  SomeModelModelIdentifier get modelIdentifier {
      return SomeModelModelIdentifier(
        id: id
      );
  }
  
  String? get name {
    return _name;
  }
  
  const SomeModel._internal({required this.id, name}): _name = name;

  // rest of class definition
}

Can you provide the output of running amplify -v and the contents of amplify/cli.json?

[Jordan-Nelson]

Can you also provide the model definition of the model you are having this issue with? I know you provided an example model definition, but seeing the actual definition might help us reproduce this and understand the use case better. Thanks.

[dJani97]

Before writing the previous comment, I added this exact SomeModel definition to the schema and generated it. So all the code I pasted was actual generated code.

I noticed that you're using the @model annotation in your definition, while I was not.

[Jordan-Nelson]

I see. I am able to reproduce that. It seems that codegen assumes that if there is an id field on a non-model type (aka a custom type) that the field will be non-null. I think the type is essentially being treated like a model type (which would require the ID field). I will mark this as a bug.

Possible workarounds:

• Rename the "id" field as this only seems to be an issue if the field is named "id".
• Modify the generated code manually

Reproduction steps:

1. Create a schema that contains a non-model type that has a field named "id". That field should not be required. For example:

type MyModel @model {
  id: ID!
  name: String
  someModel: MyType
}

type MyType {
  id: String
  name: String
}

2. Run amplify codegen models
3. Observe that there is a compilation error.