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.

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

Vue: Fix memory allocation for useAuthenticator composable #1052

Merged
merged 3 commits into from
Dec 28, 2021

Conversation

ErikCH
Copy link
Contributor

@ErikCH ErikCH commented Dec 27, 2021

Issue #, if available:
#1050

Description of changes:
Added wrapper to handle memory better when reusing the useAuthenticator composable in multiple vue instances.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@changeset-bot
Copy link

changeset-bot bot commented Dec 27, 2021

🦋 Changeset detected

Latest commit: c71c4a4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@aws-amplify/ui-vue Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@aws-amplify-us-east-2
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-1052.d3inl0muob87le.amplifyapp.com

@ErikCH ErikCH changed the title Added wrapper for useAuthenticator that handles memory better Vue: Fix memory allocation for useAuthenticator composable Dec 27, 2021
@ErikCH ErikCH temporarily deployed to ci December 27, 2021 20:13 Inactive
@ErikCH ErikCH temporarily deployed to ci December 27, 2021 20:13 Inactive
@ErikCH ErikCH temporarily deployed to ci December 27, 2021 20:13 Inactive
@ErikCH ErikCH temporarily deployed to ci December 27, 2021 20:13 Inactive
@@ -3,6 +3,7 @@ import { ref, reactive, Ref, watchEffect } from 'vue';
import { getServiceFacade } from '@aws-amplify/ui';
import { facade } from './useUtils';
import { InterpretService } from '@/components';
import { createSharedComposable } from '@vueuse/core';
Copy link
Contributor

Choose a reason for hiding this comment

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

That's an awesome library 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is! Lot's of great utilities in it!

@aws-amplify-us-east-2
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-1052.d3inl0muob87le.amplifyapp.com

@wlee221 wlee221 temporarily deployed to ci December 28, 2021 18:22 Inactive
@wlee221 wlee221 temporarily deployed to ci December 28, 2021 18:22 Inactive
@wlee221 wlee221 temporarily deployed to ci December 28, 2021 18:22 Inactive
@wlee221 wlee221 temporarily deployed to ci December 28, 2021 18:22 Inactive
@reesscot
Copy link
Contributor

Mind adding a little more detail in the description as to how this improves memory useage? Link to blog article or something?

@ErikCH
Copy link
Contributor Author

ErikCH commented Dec 28, 2021

@reesscot I showed a pattern of using external composables on my YouTube channel and a commenter mentioned that I should look into how memory is being affected, since he's seen memory leaks with composables that use reactive properties.

After further research I learned about effectScope and how it helps solve this problem by disposing computed and watch elements outside of a Vue instance.

I then looked at how the popular VueUse library was creating it's composables. It was using effectScope in several places to handle clearing out memory. I then saw it had it's own createSharedComposable utility that wrapped itself around effectScope.

After adding this into our Vue code base, it seemed that the watchEffect in useAuthenticator was being called a lot less than it did before.

@ErikCH ErikCH merged commit 5d96f73 into main Dec 28, 2021
@ErikCH ErikCH deleted the hotfix-vue-memory branch December 28, 2021 21:34
@github-actions github-actions bot mentioned this pull request Dec 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants