Skip to content
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

fix: schema aliases can now be dereferenced correctly #80

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

josh-burton
Copy link
Contributor

@josh-burton josh-burton commented Feb 10, 2025

This PR fixes an issue where schemas that alias a primitive type schema such as string or int cannot be dereferenced correctly.

@@ -59,6 +59,7 @@ bool _checkReferenceTypes(name, ref, self) {
return true;
} else if (self is SchemaObject) {
// Reference types can be different if the reference is a SchemaObject
// should this return true?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@walsha2 are you able to weigh in here at all?

I think the _checkReferenceTypes needs either a rework or a fix.

The comment on this line seems to suggest it should return true.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have attempted to implement a fix, which works for my test cases.

@josh-burton josh-burton marked this pull request as ready for review February 10, 2025 20:07
@walsha2
Copy link
Contributor

walsha2 commented Feb 12, 2025

@josh-burton I think this is working (generally) as expected. It might make more sense if you take a look at #81 which resolves a somewhat related issue when I was trying to recreate your issue.

In short, the dereference for the field myField is not a String but rather the actual type that is being referenced, MyAlias. So the dereferencing is working as expected and matches the spec.

This package does not resolve (by design) any further than the schema definitions. Since this is a primitive, the way this package resolves it during the code generation step is to explicitly create the typedef:

typedef StringAlias = String;

Then:

  const factory MyObject({
    @JsonKey(includeIfNull: false) StringAlias? myField,
  }) = _MyObject;

See #81. So yea, if you look at it from the lens of the expected generated code from this package, the dereference is correct. If you are looking to further dereference down to the primitive type it might best be done in your own code? Since you are doing your own code generation.

I might also suggest that you take the typedef approach. If an OpenAPI spec defines a type alias, IMHO this is best represented in code generation as a typedef so that the generated code is as close to a 1:1 as the spec making traceability much clearer.


Let me know if I misunderstood the issue!

@josh-burton
Copy link
Contributor Author

@josh-burton I think this is working (generally) as expected. It might make more sense if you take a look at #81 which resolves a somewhat related issue when I was trying to recreate your issue.

In short, the dereference for the field myField is not a String but rather the actual type that is being referenced, MyAlias. So the dereferencing is working as expected and matches the spec.

This package does not resolve (by design) any further than the schema definitions. Since this is a primitive, the way this package resolves it during the code generation step is to explicitly create the typedef:

typedef StringAlias = String;

Then:

  const factory MyObject({
    @JsonKey(includeIfNull: false) StringAlias? myField,
  }) = _MyObject;

See #81. So yea, if you look at it from the lens of the expected generated code from this package, the dereference is correct. If you are looking to further dereference down to the primitive type it might best be done in your own code? Since you are doing your own code generation.

I might also suggest that you take the typedef approach. If an OpenAPI spec defines a type alias, IMHO this is best represented in code generation as a typedef so that the generated code is as close to a 1:1 as the spec making traceability much clearer.

Let me know if I misunderstood the issue!

Thanks for the explanation. I'm not currently using typedefs in my generated code but I will implement it and see if it solves my issue.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants