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 issue with styled-components and webpack #44

Merged
merged 1 commit into from
Mar 3, 2023

Conversation

QuintonC
Copy link
Collaborator

@QuintonC QuintonC commented Mar 2, 2023

ℹ️ What is the context for these changes?

Addresses an issue with styled-components and webpack. The problem occurs when styled-components is used inside of an ESM only package that is used within a webpack-based bundler (e.g. CRA + Next.js).

ESM allows for importing in the syntax we're all familiar with. However, the ESM support that Next.js ships with still utilizes require instead of import. This means that the compiled styled-components code will look like styled.default.span rather than styled.span, which is why you see the error demonstrated in the before section below.

There are other approaches to resolving this which I considered, but decided against since they require more lift, such as:

  • Upgrading to an alpha package of Styled Components (Version 6 is supposed to ship with ESM support, however when testing V6 I ran into type mismatching and other weird behavior.
  • Using a different styling library (TailwindCSS, Vanilla Extract, CSS Modules)

To explain the solution here, what I've effectively done is re-exported styled-components with a "hacky" way of providing backwards-compatible CJS/ESM support. You can read more about this approach here. You can see this implementation here.

🕹️ Demonstration

State Image
Before SCR-20230302-m1h
After SCR-20230302-mz6

🎩 How can this be tophatted?

To test this, you will need a Next.js app to test with

  1. Pull this PR
  2. Install dependencies, build the packages, and then link the package dev up && yarn build && cd packages/connect-wallet && yarn link
  3. Navigate to your Next.js test application and run the following: yarn link "@shopify/connect-wallet"
  4. Run your application and you should see a Wagmi error (temporary)

✅ Checklist

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

@QuintonC QuintonC added Type: Bug 🐛 Something isn't working Dependencies Pull requests that update a dependency file Package: connect-wallet Package: tokengate labels Mar 2, 2023
@QuintonC QuintonC self-assigned this Mar 2, 2023
@QuintonC
Copy link
Collaborator Author

QuintonC commented Mar 2, 2023

/snapit

@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 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/[email protected]
yarn add @shopify/[email protected]
yarn add @shopify/[email protected]

@QuintonC QuintonC force-pushed the bugfix/styled-components-webpack-support branch from 93a4685 to 028cf37 Compare March 2, 2023 23:01
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.

We've discussed this change offline. I'm good with this change. Tophatted on desktop using MetaMask extension.

@QuintonC QuintonC mentioned this pull request Mar 3, 2023
@QuintonC QuintonC force-pushed the bugfix/styled-components-webpack-support branch from 028cf37 to 235a1ba Compare March 3, 2023 20:12
@QuintonC QuintonC merged commit 4bb50a2 into main Mar 3, 2023
@QuintonC QuintonC deleted the bugfix/styled-components-webpack-support branch March 3, 2023 20:19
@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
Dependencies Pull requests that update a dependency file Package: connect-wallet Package: tokengate Type: Bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants