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

feat(components): add twin.macro for styling #221

Closed
wants to merge 3 commits into from

Conversation

anniehu4
Copy link
Contributor

@anniehu4 anniehu4 commented Dec 9, 2020

Summary:

Builds on #224

Adds twin.macro and updates the playground example. The tw styles are copied from button.scss so they should look the same

Test Plan:

NOTE: both Jest tests & storybook are currently broken 😬

Jest error:

FAIL src/playground/button.spec.tsx
● Test suite failed to run
     TypeError: /Users/ahu/Documents/lp-design-system/packages/components/src/playground/button.tsx: Cannot destructure property 'end' of 'ex.node.loc' as it is undefined.
       at end (node_modules/@linaria/babel-preset/src/evaluators/templateProcessor.ts:123:17)
           at Array.forEach (<anonymous>)
       at forEach (node_modules/@linaria/babel-preset/src/evaluators/templateProcessor.ts:92:18)
       at process (node_modules/@linaria/babel-preset/src/extract.ts:201:41)
           at Array.forEach (<anonymous>)
       at PluginPass.forEach (node_modules/@linaria/babel-preset/src/extract.ts:201:23)
       at newFn (node_modules/@babel/core/node_modules/@babel/traverse/lib/visitors.js:175:21)
       at NodePath._call (node_modules/@babel/core/node_modules/@babel/traverse/lib/path/context.js:55:20)
       at NodePath.call (node_modules/@babel/core/node_modules/@babel/traverse/lib/path/context.js:42:17)
       at NodePath.visit (node_modules/@babel/core/node_modules/@babel/traverse/lib/path/context.js:92:31)

Storybook: same errors as #224

Once that's fixed, will confirm that:

  • Percy doesn't show visual changes on the buttons
  • npm start works
  • Jest tests pass

@@ -3,7 +3,7 @@ import { render } from "@testing-library/react";
import { variants } from "./button.stories";

describe("<Button />", () => {
it("renders the component", () => {
it.skip("renders the component", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the stories don't compile right now :/ still looking into it

"typescript": "^4.1.2"
"twin.macro": "^2.0.5",
"typescript": "^4.1.2",
"vue": "^2.6.12"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I saw warnings that vue, autoprefixer, and postcss were peer dependencies

Copy link
Contributor

Choose a reason for hiding this comment

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

we definitely shouldn't need vue - I'm guessing this is a weird peer dependency that is meant to define what versions it's compatible with. We should try to exclude it even if there are warnings.

@anniehu4 anniehu4 linked an issue Dec 9, 2020 that may be closed by this pull request
@anniehu4
Copy link
Contributor Author

anniehu4 commented Dec 9, 2020

These are the Storybook errors I'm seeing:

@chanzuckerberg/czedi-kit-components: ERROR in ./node_modules/resolve/lib/async.js
@chanzuckerberg/czedi-kit-components: Module not found: Error: Can't resolve 'fs' in '/Users/ahu/Documents/lp-design-system/packages/components/node_modules/resolve/lib'

This thread shows the same errors and warnings: ben-rogerson/twin.macro#125, and one of the answers says

Those errors pop up when twin is run through the typescript compiler rather than the babel compiler.
You'll need to replace the typescript loader with babel-loader.

It's fixed in [email protected], but I'm not sure what the version we're on (5.3.18) does

`;

return (
<Button className={variant === "primary" ? primary : secondary} {...rest} />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

there are a lot of ways to use Linaria's API -- this is based on their Theming example but isn't exactly the same. I didn't have a strong feel for what's best

Base automatically changed from ahu/babel to master December 9, 2020 21:16
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

size-limit report

Path Size
packages/components/dist/button/button.js 0 B (-100%)
packages/styles/dist/all.css 313 B (0%)
packages/components/dist/playground/button.js 997 B (+100% 🔺)

@anniehu4 anniehu4 changed the base branch from master to ahu/linaria December 10, 2020 16:47
@anniehu4 anniehu4 changed the title feat: add linaria and twin.macro for styling feat(components): add twin.macro for styling Dec 10, 2020
@anniehu4 anniehu4 marked this pull request as draft December 10, 2020 16:59
@anniehu4 anniehu4 closed this Dec 15, 2020
@booc0mtaco booc0mtaco deleted the ahu/linaria-twin branch March 8, 2023 17:35
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.

Switch to CSS-in-JS for Styling Purposes
2 participants