aws-amplify / amplify-flutter

A declarative library with an easy-to-use interface for building Flutter applications on AWS.
https://docs.amplify.aws
Apache License 2.0
1.31k stars 245 forks source link

[Codegen] CustomType and Align Standard Flutter/Dart Rules #934

Closed Peng-Qian closed 3 years ago

Peng-Qian commented 3 years ago

Hi teams,

Really love Amplify + Flutter, it truly solves many pain in Flutter, I have to say this will greatly leverage Flutter

I found following problems to use Amplify:

Problem 1: Code generation not generate custom type at all

- in SomeType.dart, there is nothing but imports, e.g.
```dart
/*
* Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
*  http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

// ignore_for_file: public_member_api_docs

import 'package:amplify_datastore_plugin_interface/amplify_datastore_plugin_interface.dart';
import 'package:flutter/foundation.dart';

Problem 2: Code generation not align to effective dart and flutter_lints

I understood it is impossible to conform to all user custom lint rules, but I think it is very necessary to align to standard and basic rules as this is a dart/flutter package

  1. DO name source files using lowercase_with_underscores. it make things so much nicer
2. cli.json: all keys in this file are case insensitive, it is really hard to read, ```dart { "features": { "graphqltransformer": { "addmissingownerfields": true, "improvepluralization": true, "validatetypenamereservedwords": true, "useexperimentalpipelinedtransformer": false, "enableiterativegsiupdates": true, "secondarykeyasgsi": true, "skipoverridemutationinputtypes": true }, "frontend-ios": { "enablexcodeintegration": true }, "auth": { "enablecaseinsensitivity": true, "useinclusiveterminology": true, "breakcirculardependency": true, "forcealiasattributes": false }, "codegen": { "useappsyncmodelgenplugin": true, "usedocsgeneratorplugin": true, "usetypesgeneratorplugin": true, "cleangeneratedmodelsdirectory": true, "retaincasestyle": true, "addtimestampfields": true, "handlelistnullabilitytransparently": true, "emitauthprovider": true, "generateindexrules": true, "enabledartnullsafety": true }, "appsync": { "generategraphqlpermissions": true }, "latestregionsupport": { "pinpoint": 1, "translate": 1, "transcribe": 1, "rekognition": 1, "textract": 1, "comprehend": 1 } } } ```
  1. prefer_const_constructors: this is quite essential since it does improve the performance!

  2. prefer_if_null_operators: this make things easier to read

  3. others

Problem 3: Custom generated file location

Jordan-Nelson commented 3 years ago

Hello @Peng-Qian - Thanks for taking the time to open this issue.

There is an open issue for Custom Types (aka non-model types) (https://github.com/aws-amplify/amplify-flutter/issues/260). This functionality was recently merged into main and I believe is just pending release. You can follow that issue for updates.

There is an open issue (https://github.com/aws-amplify/amplify-codegen/issues/510) for the issues with linting. This issue should cover everything under Problem 2, except for the naming in cli.json. I don't believe there is an open request for this. If this is something you would like to suggest a change for, can you open an issue in the Amplify CLI repo?

For problem 3, there is nothing preventing you from moving them after generating the files. I do understand how that could become tedious to do each time the files are generated. The CLI recently added functionality to run custom hooks after a CLI command is run. Although I have not attempted to do this, I believe you could use this to automatically move the files to a new location each time they are generated. Here are the docs for custom hooks. If you want to open a feature request for an alternate way to do this (cli argument, config file, etc.) can you open a feature request in the Amplify Codegen repo?

Peng-Qian commented 3 years ago

Hi @Jordan-Nelson ,

Thanks for your efficient reply!

HuiSF commented 2 years ago

Hi @Peng-Qian amplify-flutter 0.3.0-rc.2 has been release which includes the support of CustomType. The use case described in problem 1 you mentioned above is now supported.

This preview release requires a corresponding preview release of @aws-amplify/cli. Please refer to the changelog to get the detail how to start using CustomType with this preview release. Please test with your project, and follow up if you encounter any issues. Feedback is much appreciated!

Peng-Qian commented 2 years ago

Hi @HuiSF ,

It still not pull/generate any custom type in my test project with 0.3.0-rc.3

Procedure

  1. create model online
  2. upgrade cli by npm uninstall -g @aws-amplify/cli && npm install -g @aws-amplify/cli@6.2.0-custom-type-preview.0
  3. pull by amplify pull --sandboxId XXXXXXX (no custom type generated)
  4. try amplify codegen models (no custom type generated)
HuiSF commented 2 years ago

Hi @Peng-Qian , thanks for trying out the new feature.

There is a new preview release on the cli package, could you with the new version?

npm uninstall -g @aws-amplify/cli && npm install -g @aws-amplify/cli@7.7.0-flutter-preview.1

I created a new project with 0.3.0-rc.3, and the custom type can be correctly generated.

If it's still not working for you, please check the schema.graphql under <your-project-directory>/amplify/backend/api/<your-api-name>, see if the newly created CustomType is in this file.

Peng-Qian commented 2 years ago

Hi @HuiSF ,

after I upgraded cli, the custom type still not be generated and the schema.graphal is correct (it has custom type);

e.g.

type Signature {
  signerId: ID!
  timestamp: [AWSDateTime!]
}

and the model file is like

/*
* Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
*  http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

// 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

// ignore_for_file: public_member_api_docs, file_names, unnecessary_new, prefer_if_null_operators, prefer_const_constructors, slash_for_doc_comments, annotate_overrides, non_constant_identifier_names, unnecessary_string_interpolations, prefer_adjacent_string_concatenation, unnecessary_const, dead_code

import 'package:amplify_datastore_plugin_interface/amplify_datastore_plugin_interface.dart';
import 'package:flutter/foundation.dart';

I feel something might be missing, should I upgrade anything else?

HuiSF commented 2 years ago

Could you run below command to verify the cli versions @Peng-Qian

npm list -g --depth=1

Please paste the tree under @aws-amplify/cli, also please double check if you have multiple versions of @aws-amplify/cli installed in your system.

In addition, you may need to run flutter pub get to update the package versions, and check if the amplify-flutter packages are updated to 0.3.0-rc.3 version in the pubspec.lock file.

Peng-Qian commented 2 years ago

Hi @HuiSF I tried again, and still not work...

Please find npm list -g --depth=1 and pubspec.lock as follow

npm list

├─┬ @aws-amplify/cli@7.7.0-flutter-preview.1
│ ├── @aws-amplify/amplify-category-api@1.1.6
│ ├── @aws-amplify/amplify-category-auth@2.4.4
│ ├── @aws-amplify/amplify-category-custom@2.3.7
│ ├── @aws-amplify/amplify-category-storage@3.1.3
│ ├── @aws-amplify/amplify-util-uibuilder@1.1.2
│ ├── @aws-amplify/graphql-auth-transformer@0.5.3
│ ├── @aws-cdk/cloudformation-diff@1.124.0
│ ├── amplify-app@4.2.8
│ ├── amplify-category-analytics@3.2.8
│ ├── amplify-category-function@3.3.8
│ ├── amplify-category-geo@2.2.9
│ ├── amplify-category-hosting@3.2.8
│ ├── amplify-category-interactions@3.2.8
│ ├── amplify-category-notifications@2.19.4
│ ├── amplify-category-predictions@3.2.8
│ ├── amplify-category-xr@3.2.8
│ ├── amplify-cli-core@2.4.3
│ ├── amplify-cli-logger@1.1.0
│ ├── amplify-codegen@2.27.0-flutter-preview.0
│ ├── amplify-console-hosting@2.2.8
│ ├── amplify-container-hosting@2.4.6
│ ├── amplify-dotnet-function-runtime-provider@1.6.4
│ ├── amplify-dotnet-function-template-provider@2.2.9
│ ├── amplify-frontend-android@3.3.0
│ ├── amplify-frontend-flutter@1.3.0
│ ├── amplify-frontend-ios@3.3.7
│ ├── amplify-frontend-javascript@3.3.7
│ ├── amplify-go-function-runtime-provider@2.2.8
│ ├── amplify-go-function-template-provider@1.3.11
│ ├── amplify-java-function-runtime-provider@2.2.8
│ ├── amplify-java-function-template-provider@1.5.10
│ ├── amplify-nodejs-function-runtime-provider@2.2.8
│ ├── amplify-nodejs-function-template-provider@2.2.9
│ ├── amplify-prompts@1.6.0
│ ├── amplify-provider-awscloudformation@5.9.0-flutter-preview.0
│ ├── amplify-python-function-runtime-provider@2.2.8
│ ├── amplify-python-function-template-provider@1.3.13
│ ├── amplify-util-import@2.2.8
│ ├── amplify-util-mock@4.3.0-flutter-preview.0
│ ├── aws-sdk@2.1046.0
│ ├── chalk@4.1.2
│ ├── ci-info@2.0.0
│ ├── cli-table3@0.6.0
│ ├── cloudform-types@4.2.0
│ ├── colors@1.4.0
│ ├── ejs@3.1.6
│ ├── enquirer@2.3.6
│ ├── env-editor@0.5.0
│ ├── esm@3.2.25
│ ├── execa@5.1.1
│ ├── folder-hash@4.0.1
│ ├── fs-extra@8.1.0
│ ├── glob@7.2.0
│ ├── global-prefix@3.0.0
│ ├── graphql-transformer-core@7.3.0
│ ├── gunzip-maybe@1.4.2
│ ├── hidefile@3.0.0
│ ├── ini@1.3.8
│ ├── inquirer@7.3.3
│ ├── lodash@4.17.21
│ ├── node-fetch@2.6.6
│ ├── open@7.4.2
│ ├── ora@4.1.1
│ ├── parse-json@5.2.0
│ ├── progress@2.0.3
│ ├── promise-sequential@1.1.1
│ ├── semver@7.3.5
│ ├── tar-fs@2.1.1
│ ├── update-notifier@5.1.0
│ ├── uuid@3.4.0
│ ├── which@2.0.2
│ └── winston@3.3.3

pubspec.lock

packages:
  amplify_analytics_plugin_interface:
    dependency: transitive
    description:
      name: amplify_analytics_plugin_interface
      url: "https://pub.dartlang.org"
    source: hosted
    version: "0.3.0-rc.3"
  amplify_api_plugin_interface:
    dependency: transitive
    description:
      name: amplify_api_plugin_interface
      url: "https://pub.dartlang.org"
    source: hosted
    version: "0.3.0-rc.3"
  amplify_auth_plugin_interface:
    dependency: transitive
    description:
      name: amplify_auth_plugin_interface
      url: "https://pub.dartlang.org"
    source: hosted
    version: "0.3.0-rc.3"
  amplify_core:
    dependency: transitive
    description:
      name: amplify_core
      url: "https://pub.dartlang.org"
    source: hosted
    version: "0.3.0-rc.3"
  amplify_datastore:
    dependency: "direct main"
    description:
      name: amplify_datastore
      url: "https://pub.dartlang.org"
    source: hosted
    version: "0.3.0-rc.3"
  amplify_datastore_plugin_interface:
    dependency: transitive
    description:
      name: amplify_datastore_plugin_interface
      url: "https://pub.dartlang.org"
    source: hosted
    version: "0.3.0-rc.3"
  amplify_flutter:
    dependency: "direct main"
    description:
      name: amplify_flutter
      url: "https://pub.dartlang.org"
    source: hosted
    version: "0.3.0-rc.3"
  amplify_storage_plugin_interface:
    dependency: transitive
    description:
      name: amplify_storage_plugin_interface
      url: "https://pub.dartlang.org"
    source: hosted
    version: "0.3.0-rc.3"
Peng-Qian commented 2 years ago

btw, it also says

npm ERR! peer dep missing: react@^17.0.2, required by @aws-amplify/codegen-ui-react@1.2.0
npm ERR! peer dep missing: react-dom@^17.0.2, required by @aws-amplify/codegen-ui-react@1.2.0

but I think it is fine since I am using flutter

Peng-Qian commented 2 years ago

and my current flutter doctor info

Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel beta, 2.8.0-3.2.pre, on macOS 11.6 20G165 darwin-x64, locale en-NZ)
[✓] Android toolchain - develop for Android devices (Android SDK version 30.0.0-rc4)
[✓] Xcode - develop for iOS and macOS (Xcode 13.2)
[✓] Android Studio (version 2020.3)
[✓] VS Code (version 1.63.0)
[✓] Connected device (2 available)
HuiSF commented 2 years ago

Everything looks correct @Peng-Qian , know I highly suspect you may have multiple versions of the CLI installed somehow.

Let's try this:

npm uninstall -g @aws-amplify/cli
amplify --version

Do you see any version output?

Peng-Qian commented 2 years ago

actually yes, it shows 7.6.3

Peng-Qian commented 2 years ago

Hi @HuiSF ,

After I completely remove the aws-amplify/cli and reinstall the preview.1, the custom type is generated!

thx a lot!