Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): make AmplifyOutputs category types as unknown #14153

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

fix(core): make AmplifyOutputs category types as unknown #14153

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8b8c2a7
fix(core): make AmplifyOutputs category types as unknown
Jan 21, 2025
08dde45
chore: remove previously added path parsing logic for AmplifyOutputs
Jan 22, 2025
990e987
chore: remove previously added Partial on AmplifyOutputs
Jan 22, 2025
5da2834
add adapter-core internals subpath
Jan 25, 2025
3d55d3c
Merge branch 'main' into amplifyOutputs-deepPartial
ashwinkumar6 Jan 25, 2025
dd75f4e
change AmplifyOutputs deprecation message wording
Jan 29, 2025
c63d1d6
add type casting for categories with mandatory properties
Jan 29, 2025
d0f6eb4
Merge branch 'main' into amplifyOutputs-deepPartial
ashwinkumar6 Jan 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions packages/adapter-nextjs/src/types/NextServer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
import { GetServerSidePropsContext as NextGetServerSidePropsContext } from 'next';
import { NextRequest, NextResponse } from 'next/server.js';
import { cookies } from 'next/headers.js';
import { AmplifyOutputs, LegacyConfig } from 'aws-amplify/adapter-core';
import {
AmplifyOutputsUnknown,
LegacyConfig,
} from 'aws-amplify/adapter-core/internals';
import { AmplifyServer } from '@aws-amplify/core/internals/adapter-core';
import { ResourcesConfig } from '@aws-amplify/core';

Expand Down Expand Up @@ -74,7 +77,7 @@ export declare namespace NextServer {
) => Promise<OperationResult>;

export interface CreateServerRunnerInput {
config: ResourcesConfig | LegacyConfig | AmplifyOutputs;
config: ResourcesConfig | LegacyConfig | AmplifyOutputsUnknown;
}

export interface CreateServerRunnerOutput {
Expand Down
7 changes: 7 additions & 0 deletions packages/aws-amplify/adapter-core/internals/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "aws-amplify/adapter-core/internals",
"types": "../../dist/esm/adapter-core/internals.d.ts",
"main": "../../dist/cjs/adapter-core/internals.js",
"module": "../../dist/esm/adapter-core/internals.mjs",
"sideEffects": false
}
2 changes: 2 additions & 0 deletions packages/aws-amplify/src/adapter-core/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ export {
createUserPoolsTokenProvider,
} from './authProvidersFactories/cognito';
export {
/** @deprecated This type is deprecated and will be removed in future versions. */
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't be publicly exposed, updated for internal use via 'aws-amplify/adapter-core/internals'

LegacyConfig,
/** @deprecated This type is deprecated and will be removed in future versions. */
AmplifyOutputs,
} from '@aws-amplify/core/internals/utils';
export {
Expand Down
7 changes: 7 additions & 0 deletions packages/aws-amplify/src/adapter-core/internals.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

export {
LegacyConfig,
AmplifyOutputsUnknown,
} from '@aws-amplify/core/internals/utils';
4 changes: 2 additions & 2 deletions packages/aws-amplify/src/initSingleton.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
defaultStorage,
} from '@aws-amplify/core';
import {
AmplifyOutputs,
AmplifyOutputsUnknown,
LegacyConfig,
parseAmplifyConfig,
} from '@aws-amplify/core/internals/utils';
Expand All @@ -32,7 +32,7 @@ export const DefaultAmplify = {
* Amplify.configure(config);
*/
configure(
resourceConfig: ResourcesConfig | LegacyConfig | AmplifyOutputs,
resourceConfig: ResourcesConfig | LegacyConfig | AmplifyOutputsUnknown,
libraryOptions?: LibraryOptions,
): void {
const resolvedResourceConfig = parseAmplifyConfig(resourceConfig);
Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/libraryUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ export {
export { parseAWSExports } from './parseAWSExports';
export { isAmplifyOutputs, parseAmplifyOutputs } from './parseAmplifyOutputs';
export { LegacyConfig } from './singleton/types';
export { AmplifyOutputs } from './singleton/AmplifyOutputs/types';
export {
AmplifyOutputs,
AmplifyOutputsUnknown,
} from './singleton/AmplifyOutputs/types';
export { ADD_OAUTH_LISTENER } from './singleton/constants';
export { amplifyUuid } from './utils/amplifyUuid';
export { AmplifyUrl, AmplifyUrlSearchParams } from './utils/amplifyUrl';
Expand Down
50 changes: 25 additions & 25 deletions packages/core/src/parseAmplifyOutputs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {
} from './singleton/Auth/types';
import { NotificationsConfig } from './singleton/Notifications/types';
import {
AmplifyOutputs,
AmplifyOutputsAnalyticsProperties,
AmplifyOutputsAuthProperties,
AmplifyOutputsCustomProperties,
Expand All @@ -28,6 +27,7 @@ import {
AmplifyOutputsNotificationsProperties,
AmplifyOutputsStorageBucketProperties,
AmplifyOutputsStorageProperties,
AmplifyOutputsUnknown,
} from './singleton/AmplifyOutputs/types';
import {
AnalyticsConfig,
Expand All @@ -40,10 +40,10 @@ import {
} from './singleton/types';

export function isAmplifyOutputs(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sanity check: have we verified this change wouldnt affect if someone extracted type by using Parameters<typeof ...>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However customers can continue using the now deprecated AmplifyOutputs types from "aws-amplify/adapter-core"

Screenshot 2025-01-29 at 5 12 44 PM

config: ResourcesConfig | LegacyConfig | AmplifyOutputs,
): config is AmplifyOutputs {
config: ResourcesConfig | LegacyConfig | AmplifyOutputsUnknown,
): config is AmplifyOutputsUnknown {
// version format initially will be '1' but is expected to be something like x.y where x is major and y minor version
const { version } = config as AmplifyOutputs;
const { version } = config as AmplifyOutputsUnknown;

if (!version) {
return false;
Expand Down Expand Up @@ -291,32 +291,44 @@ function parseNotifications(
}

export function parseAmplifyOutputs(
amplifyOutputs: AmplifyOutputs,
amplifyOutputs: AmplifyOutputsUnknown,
): ResourcesConfig {
const resourcesConfig: ResourcesConfig = {};

if (amplifyOutputs.storage) {
resourcesConfig.Storage = parseStorage(amplifyOutputs.storage);
resourcesConfig.Storage = parseStorage(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, a type predicate should allow us to drop tha casting?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discussed async, decided to go ahead with type casting in this PR with a follow-up item to add run-time checks during parsing

amplifyOutputs.storage as AmplifyOutputsStorageProperties,
);
}

if (amplifyOutputs.auth) {
resourcesConfig.Auth = parseAuth(amplifyOutputs.auth);
resourcesConfig.Auth = parseAuth(
amplifyOutputs.auth as AmplifyOutputsAuthProperties,
);
}

if (amplifyOutputs.analytics) {
resourcesConfig.Analytics = parseAnalytics(amplifyOutputs.analytics);
resourcesConfig.Analytics = parseAnalytics(
amplifyOutputs.analytics as AmplifyOutputsAnalyticsProperties,
);
}

if (amplifyOutputs.geo) {
resourcesConfig.Geo = parseGeo(amplifyOutputs.geo);
resourcesConfig.Geo = parseGeo(
amplifyOutputs.geo as AmplifyOutputsGeoProperties,
);
}

if (amplifyOutputs.data) {
resourcesConfig.API = parseData(amplifyOutputs.data);
resourcesConfig.API = parseData(
amplifyOutputs.data as AmplifyOutputsDataProperties,
);
}

if (amplifyOutputs.custom) {
const customConfig = parseCustom(amplifyOutputs.custom);
const customConfig = parseCustom(
amplifyOutputs.custom as AmplifyOutputsCustomProperties,
);

if (customConfig && 'Events' in customConfig) {
resourcesConfig.API = { ...resourcesConfig.API, ...customConfig };
Expand All @@ -325,7 +337,7 @@ export function parseAmplifyOutputs(

if (amplifyOutputs.notifications) {
resourcesConfig.Notifications = parseNotifications(
amplifyOutputs.notifications,
amplifyOutputs.notifications as AmplifyOutputsNotificationsProperties,
);
}

Expand Down Expand Up @@ -383,22 +395,10 @@ function createBucketInfoMap(
);
}

const sanitizedPaths = paths
? Object.entries(paths).reduce<
Record<string, Record<string, string[] | undefined>>
>((acc, [key, value]) => {
if (value !== undefined) {
acc[key] = value;
}

return acc;
}, {})
: undefined;

mappedBuckets[name] = {
bucketName,
region,
paths: sanitizedPaths,
paths,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed previously added type narrowing

};
},
);
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/singleton/Amplify.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { deepFreeze } from '../utils';
import { parseAmplifyConfig } from '../libraryUtils';

import {
AmplifyOutputs,
AmplifyOutputsUnknown,
AuthConfig,
LegacyConfig,
LibraryOptions,
Expand Down Expand Up @@ -49,7 +49,7 @@ export class AmplifyClass {
* @param libraryOptions - Additional options for customizing the behavior of the library.
*/
configure(
resourcesConfig: ResourcesConfig | LegacyConfig | AmplifyOutputs,
resourcesConfig: ResourcesConfig | LegacyConfig | AmplifyOutputsUnknown,
libraryOptions?: LibraryOptions,
): void {
const resolvedResourceConfig = parseAmplifyConfig(resourcesConfig);
Expand Down
16 changes: 14 additions & 2 deletions packages/core/src/singleton/AmplifyOutputs/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export interface AmplifyOutputsAuthProperties {
unauthenticated_identities_enabled?: boolean;
mfa_configuration?: string;
mfa_methods?: string[];
groups?: Partial<Record<UserGroupName, UserGroupPrecedence>>[];
groups?: Record<UserGroupName, UserGroupPrecedence>[];
}

export interface AmplifyOutputsStorageBucketProperties {
Expand All @@ -53,7 +53,7 @@ export interface AmplifyOutputsStorageBucketProperties {
/** Region for the bucket */
aws_region: string;
/** Paths to object with access permissions */
paths?: Partial<Record<string, Record<string, string[] | undefined>>>;
paths?: Record<string, Record<string, string[] | undefined>>;
}
export interface AmplifyOutputsStorageProperties {
/** Default region for Storage */
Expand Down Expand Up @@ -115,6 +115,7 @@ export interface AmplifyOutputsNotificationsProperties {
channels: string[];
}

/** @deprecated Use {@link AmplifyOutputsUnknown} instead. */
export interface AmplifyOutputs {
version?: string;
storage?: AmplifyOutputsStorageProperties;
Expand All @@ -125,3 +126,14 @@ export interface AmplifyOutputs {
custom?: AmplifyOutputsCustomProperties;
notifications?: AmplifyOutputsNotificationsProperties;
}

export interface AmplifyOutputsUnknown {
version?: string;
storage?: unknown;
auth?: unknown;
analytics?: unknown;
geo?: unknown;
data?: unknown;
custom?: unknown;
notifications?: unknown;
}
2 changes: 1 addition & 1 deletion packages/core/src/singleton/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
import { NotificationsConfig } from './Notifications/types';
import { InteractionsConfig } from './Interactions/types';

export { AmplifyOutputs } from './AmplifyOutputs/types';
export { AmplifyOutputsUnknown } from './AmplifyOutputs/types';

/**
* Compatibility type representing the Amplify Gen 1 configuration file schema. This type should not be used directly.
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/utils/parseAmplifyConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

import { ResourcesConfig } from '../index';
import { AmplifyOutputs } from '../singleton/AmplifyOutputs/types';
import { AmplifyOutputsUnknown } from '../singleton/AmplifyOutputs/types';
import { LegacyConfig } from '../singleton/types';
import { parseAWSExports } from '../parseAWSExports';
import { isAmplifyOutputs, parseAmplifyOutputs } from '../parseAmplifyOutputs';
Expand All @@ -14,7 +14,7 @@ import { isAmplifyOutputs, parseAmplifyOutputs } from '../parseAmplifyOutputs';
* @return A ResourcesConfig for the provided configuration object.
*/
export const parseAmplifyConfig = (
amplifyConfig: ResourcesConfig | LegacyConfig | AmplifyOutputs,
amplifyConfig: ResourcesConfig | LegacyConfig | AmplifyOutputsUnknown,
): ResourcesConfig => {
if (Object.keys(amplifyConfig).some(key => key.startsWith('aws_'))) {
return parseAWSExports(amplifyConfig);
Expand Down
Loading