fix(core): make AmplifyOutputs category types as unknown #14153
Conversation
ashwinkumar6
requested review from
sktimalsina,
cshfang,
HuiSF,
pranavosu and
a team
as code owners
| mappedBuckets[name] = { | ||
| bucketName, | ||
| region, | ||
| paths: sanitizedPaths, | ||
| paths, |
There was a problem hiding this comment.
removed previously added type narrowing
| @@ -296,27 +296,39 @@ export function parseAmplifyOutputs( | |||
| const resourcesConfig: ResourcesConfig = {}; | |||
|
|
|||
| if (amplifyOutputs.storage) { | |||
| resourcesConfig.Storage = parseStorage(amplifyOutputs.storage); | |||
| resourcesConfig.Storage = parseStorage( | |||
There was a problem hiding this comment.
i dont think type casting is the best way here. IMO we should add guard rails in the parse* functions to make sure types hold and throw errors earlier if something is messed up
There was a problem hiding this comment.
+1, a type predicate should allow us to drop tha casting?
There was a problem hiding this comment.
discussed async, decided to go ahead with type casting in this PR with a follow-up item to add run-time checks during parsing
ashwinkumar6
force-pushed
the
amplifyOutputs-deepPartial
branch
from
0d1cbae to
5da2834
Compare
| @@ -8,7 +8,9 @@ export { | |||
| createUserPoolsTokenProvider, | |||
| } from './authProvidersFactories/cognito'; | |||
| export { | |||
| /** @deprecated This type is deprecated and will be removed in future versions. */ | |||
There was a problem hiding this comment.
Shouldn't be publicly exposed, updated for internal use via 'aws-amplify/adapter-core/internals'
| @@ -40,10 +40,10 @@ import { | |||
| } from './singleton/types'; | |||
|
|
|||
| export function isAmplifyOutputs( | |||
There was a problem hiding this comment.
sanity check: have we verified this change wouldnt affect if someone extracted type by using Parameters<typeof ...>
There was a problem hiding this comment.
Amplify.configure no longer users AmplifyOutputs type, instead it uses AmplifyOutputsUnknown. That means
customers will not be able to extract inner nested AmplifyOutputs category properties
import { AmplifyOutputs } from "aws-amplify/adapter-core";
type AmplifyOutputsAuth = Extract<
Parameters<typeof Amplify.configure>[0],
AmplifyOutputs
>["auth"];
type AuthOauth = NonNullable<AmplifyOutputsAuth>["oauth"];
In the example above post our change AuthOauth would be never
There was a problem hiding this comment.
However customers can continue using the now deprecated AmplifyOutputs types from "aws-amplify/adapter-core"
Description of changes
Solves issues where Typescript inferred type from
amplify_outputs.jsonfile is strongly typed when compared to AmplifyOutputs type. Updates AmplifyOutputs type to make all category key's asunknown.Other changes:
AmplifyOutputstype toAmplifyOutputsLegacyand add a deprecating warning.AmplifyOutputstype publicly exposed from "aws-amplify/adapter-core" will exposeAmplifyOutputsLegacyunder the same name instead to avoid any breaking change.LegacyConfigandAmplifyOutputstypes from "aws-amplify/adapter-core"Issue #, if available
A long term solution for issues like #14045, #14106
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.