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

➖ Removes redux-logger and makes use of custom logger middleware #47

Merged
merged 2 commits into from
Mar 3, 2023

Conversation

QuintonC
Copy link
Collaborator

@QuintonC QuintonC commented Mar 3, 2023

⚠️ Related: #45

ℹ️ What is the context for these changes?

Removes redux-logger from the package and utilizes our own custom logger middleware. The source for this implementation is a simplified and slimmed down version of the redux-logger code that not only matches the types of our store, but also one that we have more control over.

🕹️ Demonstration

State (w/ Demo) Image
Before SCR-20230302-ng5
After Screenshot 2023-03-03 at 2 43 07 PM

🎩 How can this be tophatted?

  • Utilize the demo links above to see the package functioning in Next.js as expected.

✅ Checklist

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

@QuintonC QuintonC added Type: Bug 🐛 Something isn't working SEV-3 Normal Severity Dependencies Pull requests that update a dependency file Area: Testing 🧪 Improvements or additions to unit tests labels Mar 3, 2023
@QuintonC QuintonC self-assigned this Mar 3, 2023
@QuintonC QuintonC force-pushed the chore/remove-redux-logger branch 2 times, most recently from 66af839 to 42b10c5 Compare March 3, 2023 20:22
@QuintonC
Copy link
Collaborator Author

QuintonC commented Mar 3, 2023

/snapit

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2023

🫰✨ Thanks @QuintonC! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/connect-wallet@0.0.0-snapshot-20230303202422
yarn add @shopify/gate-context-client@0.0.0-snapshot-20230303202422
yarn add @shopify/tokengate@0.0.0-snapshot-20230303202422

@QuintonC QuintonC marked this pull request as ready for review March 3, 2023 20:43
@QuintonC QuintonC linked an issue Mar 3, 2023 that may be closed by this pull request
Copy link
Contributor

@jamiely jamiely left a comment

Choose a reason for hiding this comment

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

👍

console.groupEnd();
} catch (error) {
console.log('—— log end ——');
}
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 this looks much simpler than what was there before. I'm surprised that the tests didn't break.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Haha what makes you think the tests would've broken? Previously we were just pushing logs into an array that would only ever have a single entry (from what I can tell), so it makes sense to me that it wouldn't have broken.

@QuintonC QuintonC merged commit 7fc9f55 into main Mar 3, 2023
@QuintonC QuintonC deleted the chore/remove-redux-logger branch March 3, 2023 22:55
@github-actions github-actions bot 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
Labels
Area: Testing 🧪 Improvements or additions to unit tests Dependencies Pull requests that update a dependency file SEV-3 Normal Severity Type: Bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build issue with nextJS
2 participants