Skip to content

chore: Clean up lingering usage of deprecated default exports. #4790

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

Merged
merged 2 commits into from
Nov 7, 2022
Merged

chore: Clean up lingering usage of deprecated default exports. #4790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e624a55
chore: Clean up lingering usage of deprecated default exports.
jimblanc Nov 7, 2022
9ae9831
Cleaned up missed import.
jimblanc Nov 7, 2022
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
3 changes: 2 additions & 1 deletion src/fragments/lib-v1/predictions/js/getting-started.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ Import and load the configuration file in your app. It's recommended you add the

```javascript
import { Amplify } from 'aws-amplify';
import Predictions, {
import {
Predictions,
AmazonAIPredictionsProvider
} from '@aws-amplify/predictions';
import awsconfig from './aws-exports';
Expand Down
2 changes: 1 addition & 1 deletion src/fragments/lib-v1/push-notifications/js/getting-started.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ If you don't have Analytics already enabled, see our [Analytics Developer Guide]

```javascript
import { Amplify } from 'aws-amplify';
import PushNotification from '@aws-amplify/pushnotification';
import { PushNotification } from '@aws-amplify/pushnotification';
import PushNotificationIOS from '@react-native-community/push-notification-ios';
import awsconfig from './aws-exports';

Expand Down
3 changes: 2 additions & 1 deletion src/fragments/lib/predictions/js/getting-started.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ Import and load the configuration file in your app. It's recommended you add the

```javascript
import { Amplify } from 'aws-amplify';
import Predictions, {
import {
Predictions,
AmazonAIPredictionsProvider
} from '@aws-amplify/predictions';
import awsconfig from './aws-exports';
Expand Down
2 changes: 1 addition & 1 deletion src/fragments/lib/push-notifications/js/getting-started.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ If you don't have Analytics already enabled, see our [Analytics Developer Guide]

```javascript
import { Amplify } from 'aws-amplify';
import PushNotification from '@aws-amplify/pushnotification';
import { PushNotification } from '@aws-amplify/pushnotification';
import PushNotificationIOS from '@react-native-community/push-notification-ios';
import awsconfig from './aws-exports';

Expand Down
3 changes: 2 additions & 1 deletion src/fragments/ui-legacy/vue/overview.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ npm i aws-amplify-vue
Then, alter main.js:

```javascript
import Amplify, * as AmplifyModules from 'aws-amplify'
import { Amplify } from 'aws-amplify'
import * as AmplifyModules from 'aws-amplify'
import { AmplifyPlugin } from 'aws-amplify-vue'
import awsconfig from './aws-exports'
Amplify.configure(awsconfig)
Comment on lines +17 to 21
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit

Suggested change
import { Amplify } from 'aws-amplify'
import * as AmplifyModules from 'aws-amplify'
import { AmplifyPlugin } from 'aws-amplify-vue'
import awsconfig from './aws-exports'
Amplify.configure(awsconfig)
import * as AmplifyModules from 'aws-amplify'
import { AmplifyPlugin } from 'aws-amplify-vue'
import awsconfig from './aws-exports'
AmplifyModules.Amplify.configure(awsconfig)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I went back and forth on this one, seemed cleaner to just have two imports (to have the standard Amplify.configure call) but willing to change if you have strong opinions about it.

Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need both? what does the * as AmplifyModules accomplish here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So with default exports you're able to import the default and also star import the rest of the module, e.g. the original code: import Amplify, * as AmplifyModules from 'aws-amplify'

This really isn't possible with named exports, so something like this is not valid for example: import { Amplify }, * as AmplifyModules from 'aws-amplify'. We can do what Chris suggested, but I personally thought having AmplifyModules.Amplify.configure(awsconfig) felt weirder than just have two imports so we can use the standard Amplify.configure form which is used everywhere else. Can change though if folks feel strongly about it.

Expand Down