(chore): Added early error message for configuration error #14222
Conversation
yuhengshs
force-pushed
the
feat-amplify/error
branch
from
9f6a2d9 to
2f70090
Compare
yuhengshs
requested review from
cshfang,
HuiSF,
ashika112 and
a team
as code owners
There was a problem hiding this comment.
The amount of change here makes me wonder if we're not missing something. How do we know that we haven't missed protecting a call site that uses config? Since this is spread over all of the categories independently, are we retesting all categories to ensure this behaves as expected in each case?
| @@ -11,6 +11,7 @@ import { | |||
| import { DEFAULT_KINESIS_FIREHOSE_CONFIG } from './constants'; | |||
|
|
|||
| export const resolveConfig = () => { | |||
| Amplify.assertConfigured(); | |||
There was a problem hiding this comment.
Quite a few of these occur right before we call Amplify.getConfig(). Do we always want to assert configured with getConfig? If so, could we include this in the getConfig behavior to remove some of the amount of change happening here?
Also, there are a number of places where this isn't couple with getConfig. This seems counterintuitive, right? Are these other callsites still calling getConfig, but a few layers of abstraction removed? Is there a pattern for accessing the config different from using getConfig?
Description of changes
The purpose of this PR is to add early error message when
Amplify.configure()is not called, an error will be thrown early to the customers and notify them Amplify.configure() is not called.Unit test is added accordingly.
Issue #, if available
Description of how you validated changes
Checklist
yarn testpassesChecklist for repo maintainers
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.