search

aws-amplify / amplify-codegen

Amplify Codegen is a JavaScript toolkit library for frontend and mobile developers building Amplify applications.
Apache License 2.0
58 stars 58 forks source link

Amplify Codegen does not generate output in a consistent/deterministic order #167

Open danrivett opened 3 years ago

danrivett commented 3 years ago

First of all, I'm surprised I couldn't find an existing bug, so if I've missed it, apologies. But as I couldn't find one I'm creating this ticket as as surely I'm not the only one this drives nuts over time...

Describe the bug Running amplify push and regenerating GraphQL code bindings, or simply running amplify codegen on a GraphQL schema that hasn't changed results in unnecessary changes to files such as graphql/schema.json simply due to reordering of the same output due to apparent non-deterministic ordering in the generated code.

Amplify CLI Version 4.35.0

To Reproduce Run the commands listed above a few times and see unnecessary and noisy changes made to files such as graphql/schema.json.

Expected behavior All code generated should be generated in a consistent deterministic, possibly sorted, order so that:

  1. It is clear that a change in the generated file is caused by a change to the GraphQL schema that has just been pushed up, rather than changes in generated files unrelated to the schema change.
    1. Currently I have to either spend a lot of time eyeballing the diffs to and reverting hunks if I'm absolutely sure something didn't change, or otherwise include it in file history just to be safe.
    2. This is especially true because updating the @aws-amplify/cli package can cause expected changes to the generated files that are required even if not caused directly by user changes to their GraphQL schema, and so running a codegen after updating the package version without any GraphQL schema changes should be able to show if the version has introduced changes or not; currently that's not clear, it could just be non-deterministic reordering.
  2. Git Commit history is not polluted with a ton of unnecessary changes making it hard to browse file history.

Screenshots An example diff highlighting one such apparently unnecessary reordering:

amplify-codegen-reordering

Desktop (please complete the following information):

Additional context Git commit history is important to in order to understand when particular bugs may have been introduced and by what changes, and so unnecessary noise in codegen files is more important than it may seem.

danrivett commented 3 years ago

This issue has been bugging me for at least 6 months but I thought it would have bugged Amplify devs more with their running codegen more often than I do, so I just assumed it would be addressed for their sanity's sake if not mine 😁

I guess it's largely client codebases that would be checking these files into source control etc.

danrivett commented 3 years ago

I can also confirm this affects not just GraphQL generated files such as graphql/schema.json when running amplify codegen but also DataStore bindings generated from running amplify codegen models (e.g. index.js, index.d.ts etc.).

Again, may sound trivial, but every change I make (such as just now adding in a new Lambda DataSource) causes a lot of reordering of existing elements and it's just too dangerous most of the time to not check them in, in case there are subtle changes made somewhere.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

danrivett commented 3 years ago

Not sure if there is any update on this ticket. It would be great from an infrastructure-as-code perspective if generated code was generated in a way that removed the noise of unchanged but moved code since it's committed into source control.

I don't know how feasible that is?

pr0g commented 5 months ago

I've definitely run into this too, and see the same thing that "aws_api_key", "aws_iam" and "aws_cognito_user_pools" seem to shuffle around a lot which can be super confusing. I'd be super happy if the sort order could be well defined.

Just as a random example I've used JSON for Modern C++ in another project and it provides the type ordered_json which is perfect for these kinds of situations (see https://json.nlohmann.me/api/ordered_json/). If Amplify could use a similar mechanism inside whatever JSON library it's using that would be incredibly helpful! 🙂