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

🐛 Addresses an issue with exported types from shared #31

Merged
merged 2 commits into from
Feb 27, 2023

Conversation

QuintonC
Copy link
Collaborator

@QuintonC QuintonC commented Feb 27, 2023

⚠️ Fixes:

ℹ️ What is the context for these changes?

Dawn, Default, and Theme were exported without any information since packages/shared is not a published package.

This leads to issues when attempting to pass a theme value to not only Tokengate, but also ConnectWalletProvider. Additionally, this makes it impossible to understand the type definition of Theme as well as results in errors when attempting to override some values in any of the provided themes.

🕹️ Demonstration

Package Name Before After
@shopify/connect-wallet image image
@shopify/tokengate image image

✅ Checklist

  • Tested on mobile N/A
  • Tested on multiple browsers N/A
  • Tested for accessibility N/A
  • Includes unit tests N/A
  • Updated relevant documentation for the changes (if necessary) N/A

@QuintonC QuintonC self-assigned this Feb 27, 2023
Dawn, Default, and Theme were exported without any information since Shared is not a published package. This leads to issues when attempting to pass a theme value to  not only the base props for Tokengate and ConnectWalletProvider, but also makes it impossible to understand the type definition of Theme.
Copy link
Contributor

@caropinzonsilva caropinzonsilva left a comment

Choose a reason for hiding this comment

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

Looks good!

@QuintonC QuintonC merged commit 6d1710f into main Feb 27, 2023
@QuintonC QuintonC deleted the bugfix/address-shared-types branch February 27, 2023 23:37
@github-actions github-actions bot mentioned this pull request Feb 27, 2023
@QuintonC QuintonC mentioned this pull request Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants