amplify-js icon indicating copy to clipboard operation
amplify-js copied to clipboard

fix: Kebab case to Camel case config props

Open nadetastic opened this issue 1 year ago • 1 comments

Description of changes

  • Geo Location services access config keys with kebab case, causing manually configured resources to fail. Updating the access to use camelCase as expected. (https://github.com/aws-amplify/amplify-js/issues/12870)

  • Typo in comment under I18n (https://github.com/aws-amplify/amplify-js/issues/12752)

Issue #, if available

https://github.com/aws-amplify/amplify-js/issues/12870 https://github.com/aws-amplify/amplify-js/issues/12752

Description of how you validated changes

  • Tested with local build

Checklist

  • [ ] PR description included
  • [ ] yarn test passes
  • [ ] Tests are changed or added
  • [ ] Relevant documentation is changed or added (and PR referenced)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

nadetastic avatar Jan 29 '24 16:01 nadetastic

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (278776f) 88.02% compared to head (38164f8) 87.28%. Report is 5 commits behind head on main.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12921      +/-   ##
==========================================
- Coverage   88.02%   87.28%   -0.74%     
==========================================
  Files         663      635      -28     
  Lines       18141    13446    -4695     
  Branches     3635     2462    -1173     
==========================================
- Hits        15969    11737    -4232     
+ Misses       2172     1709     -463     
Flag Coverage Δ
adapter-nextjs 98.64% <ø> (ø)
analytics 62.75% <ø> (ø)
api ∅ <ø> (∅)
api-graphql 87.71% <ø> (ø)
api-rest 95.95% <ø> (-0.33%) :arrow_down:
auth 86.78% <ø> (ø)
aws-amplify 100.00% <ø> (ø)
core 87.72% <ø> (-0.31%) :arrow_down:
datastore ?
datastore-storage-adapter 88.13% <ø> (ø)
geo 90.12% <ø> (ø)
interactions 96.10% <ø> (ø)
notifications 90.35% <ø> (ø)
predictions 86.00% <ø> (ø)
pubsub 94.70% <ø> (ø)
rtn-push-notification 100.00% <ø> (ø)
storage 91.28% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Jan 29 '24 16:01 codecov-commenter

Think that the changes in AmazonLocationServiceProvider are non-breaking, but the fact that this is only an issue for Gen2 makes me concerned that there is something off with the returned shape of Geo in parseAWSExports as existing apps using v6 do not exhibit the issue that this PR is addressing. Before approving:

  • Need to understand why non-Gen2 customers are not facing this issue
  • Need to see a successful run of the UI e2e tests with this change

@calebpollman I wouldn’t classify it as a Gen2 vs Gen1 thing but more of Legacy vs Resource config issue:

  • Only happens when the Geo resources are manually added as ResourceConfig (camelCase) and does not happen if added as Legacy config (kebab_case)
  • The library (or really AmazonLocationServiceProvider) is expecting it to be in kebab_case (aka legacy config) however it is in camelCase (aka Resource config)
  • An important note (why it works when parseExports is used on LegacyConfig) - there are two object created when legacy config is used - one with camelCase and a copy in kebab_case, however, if Resource config is used, the legacy config object is not present which results in the error

Working on the the UI e2e tests

nadetastic avatar Feb 20 '24 19:02 nadetastic

@calebpollman I wouldn’t classify it as a Gen2 vs Gen1 thing but more of Legacy vs Resource config issue:

  • Only happens when the Geo resources are manually added as ResourceConfig (camelCase) and does not happen if added as Legacy config (kebab_case)
  • The library (or really AmazonLocationServiceProvider) is expecting it to be in kebab_case (aka legacy config) however it is in camelCase (aka Resource config)
  • An important note (why it works when parseExports is used on LegacyConfig) - there are two object created when legacy config is used - one with camelCase and a copy in kebab_case, however, if Resource config is used, the legacy config object is not present which results in the error

Working on the the UI e2e tests

From what i am reading here, the fact that parseAWSExports returns both camelCase and kebab_case objects is part of the root issue since the return does not match the expected shape of the ResourcesConfig, and should be addressed as a part of this PR to align handling of projects with hand rolled ResourcesConfig and legacy configs.

Once the handling of the Geo config parsing is fixed, the updates here, here and here to the differing inputs should be fine, but we should make sure that we do need to include lookup against the search_indices as well to ensure backwards compatibility.

calebpollman avatar Feb 20 '24 21:02 calebpollman

Updated PR to address the issue where parseAWSExports was returning both searchIndices and search_indices

nadetastic avatar Mar 26 '24 22:03 nadetastic

Close this request since it has been replaced by #13303 and released

AllanZhengYP avatar Apr 30 '24 07:04 AllanZhengYP