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

Babel plugin #62

Merged
merged 9 commits into from
Jul 25, 2017
Merged

Babel plugin #62

merged 9 commits into from
Jul 25, 2017

Conversation

SaraVieira
Copy link
Member

Closes #13 and creates a section in advanced with the babel plugin docs

"title": "Extending styles"
}, {
},
Copy link
Member Author

Choose a reason for hiding this comment

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

Prettier auto did this , should I revert ?

/>
</DocsLayout>
)
<NextPage href="/docs/api" title="API Reference" />
Copy link
Member Author

Choose a reason for hiding this comment

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

Prettier again

Copy link
Member

Choose a reason for hiding this comment

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

No worries! I might add prettier soon anyway

Copy link
Member Author

Choose a reason for hiding this comment

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

Awesome ! I can do that if you want 👍

@kitten
Copy link
Member

kitten commented Jul 23, 2017

This is looking good! 🎉

I'm thinking, maybe we should move the section to API. What do you think, @mxstbr?

@SaraVieira
Copy link
Member Author

@philpl I was on the fence between advanced and API so input would be great

@mxstbr
Copy link
Member

mxstbr commented Jul 23, 2017

Awesomeee!!

Hmm, I quite like it under Advanced but maybe we should just have another section "Tooling" or something that also includes some docs for jest-styled-components et al.? /cc @MicheleBertoli

@kitten
Copy link
Member

kitten commented Jul 23, 2017

@mxstbr Then let's put it into the "Advanced" section until we have enough content to create one for "Tooling"?

@mxstbr
Copy link
Member

mxstbr commented Jul 23, 2017

SGTM

@MicheleBertoli
Copy link
Member

Thanks @SaraVieira for your contribution. 👏

@mxstbr @philpl A "tooling" section sounds great, and I'll be happy to add more information about testing. Should I open an issue?

Also, I'm glad to see the snapshots are working on this PR :)

@kitten
Copy link
Member

kitten commented Jul 23, 2017

@MicheleBertoli I still have to add a .travis.yml but it seems like it worked out great.

An issue would be greatly appreciated :)

@SaraVieira
Copy link
Member Author

@philpl @mxstbr @MicheleBertoli Should we leave it like this or create the tooling section already ?

Copy link
Member

@kitten kitten left a comment

Choose a reason for hiding this comment

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

Some minor changes that we'll need to do. Most of the mistakes come from the original readme, but need to be fixed before they just go into the website as well 😉

styles and gives you a nicer debugging experience when using{' '}
<Code>styled-components</Code>.
</p>
<p>Usage</p>
Copy link
Member

Choose a reason for hiding this comment

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

This subsection can use the SectionLayout component with the sub prop, like the ones below.

Also, this is missing a small explanation, like:

Install the babel-plugin first:

Copy link
Member Author

Choose a reason for hiding this comment

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

Done 🌮

<p>
This plugin adds support for server-side rendering, for minification of
styles and gives you a nicer debugging experience when using{' '}
<Code>styled-components</Code>.
Copy link
Member

@kitten kitten Jul 23, 2017

Choose a reason for hiding this comment

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

You can leave out "when using styled-components" as that's quite obvious on the site. But we should add a paragraph that it's only for the web, not for React Native

Copy link
Member Author

Choose a reason for hiding this comment

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

Done 🎉

<p>Usage</p>
<CodeBlock code={installNPM} language="node" />
<p>
Then in your babel configuration (probably <Code>.babelrc</Code>):
Copy link
Member

Choose a reason for hiding this comment

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

This sounds a bit odd. Maybe something like:

Then add it to your babel configuration ([...]) like so:

would make it nicer

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed 👍

By adding a unique identifier to every styled component this plugin
avoids checksum mismatches due to different class generation on the
client and on the server. If you do not use this plugin and try to
server-side render <Code>styled-components</Code> React will complain.
Copy link
Member

Choose a reason for hiding this comment

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

<Code>styled-components</Code> needs to be changed to just styled components. We're not talking about the lib 😄

If you do not use this plugin and try to server-side render styled-components React will complain.

This is a bit outdated and also sounds weird, so let's update it:

If you do not use this plugin and try to server-side render, you will likely encounter React warnings, if the import order is different on the client. For this reason we generally recommend you to turn on this option in any SSR setup.

We also need to add a reference to this babel-plugin option to our SSR section.

Copy link
Member Author

Choose a reason for hiding this comment

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

We also need to add a reference to this babel-plugin option to our SSR section. -> What can I do here ?

Copy link
Member

Choose a reason for hiding this comment

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

I can do this, if it's unclear. In the SSR section on the website, it'd just be useful to say "You will probably need to use our babel plugin's ssr option to not run into checksum mismatches with server-side rendering."

server-side render <Code>styled-components</Code> React will complain.
</p>
<p>
If you want server-side rendering support you can enable it with the{' '}
Copy link
Member

Choose a reason for hiding this comment

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

We can cut this short due to the prior intro:

If you want server-side rendering support You can enable it with the ssr option:

Copy link
Member Author

Choose a reason for hiding this comment

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

Done 🎉

</p>
<CodeBlock code={preprocess} language="node" />
</SectionLayout>
<SectionLayout sub title="Preprocessing">
Copy link
Member

Choose a reason for hiding this comment

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

Incorrect title 😉

Copy link
Member Author

Choose a reason for hiding this comment

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

Damn , fixed :p

</SectionLayout>
<SectionLayout sub title="Preprocessing">
<Note>
This option is turned on by default! If you experience mangled CSS
Copy link
Member

Choose a reason for hiding this comment

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

We can lose the exclamation mark here

Copy link
Member Author

Choose a reason for hiding this comment

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

Done 😄

</Note>
<p>
This plugin minifies your styles in the tagged template literals, giving
you big bundle size savings. (note that you will not see the effect of
Copy link
Member

Choose a reason for hiding this comment

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

We can lose the parantheses here

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed 💃

</p>
<CodeBlock code={minify} language="node" />
<p>
We also transpile <Code>styled-components</Code> tagged template
Copy link
Member

Choose a reason for hiding this comment

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

This should go into a new section, since it's a separate option actually.

Copy link
Member Author

Choose a reason for hiding this comment

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

What should go in this section ? Just this paragraph ?

Copy link
Member

Choose a reason for hiding this comment

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

Yea, this entire part about the template string transpilation

Copy link
Member Author

Choose a reason for hiding this comment

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

screen shot 2017-07-24 at 12 44 40

Done 👍

literals down to a smaller representation than what Babel normally does,
because <Code>styled-components</Code> template literals don't need to
be 100% spec compliant. see{' '}
<a href="https://github.com/styled-components/babel-plugin-styled-components/blob/master/minification.md">
Copy link
Member

Choose a reason for hiding this comment

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

The link here can be swapped out for a link to our "Tagged Template Literals" section.

Also there's a trailing closed parenthesis below.

Copy link
Member Author

Choose a reason for hiding this comment

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

Link swapped :)

@SaraVieira
Copy link
Member Author

@philpl Made some of the changes

@kitten
Copy link
Member

kitten commented Jul 24, 2017

@SaraVieira Thanks a lot again already! This is already so close to be shippable in such a short time 😄 🎉

@SaraVieira
Copy link
Member Author

SaraVieira commented Jul 24, 2017

@philpl All done I think 🎆

Sent from my Bq Aquaris M5 using FastHub

Copy link
Member

@kitten kitten left a comment

Choose a reason for hiding this comment

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

Last problem :)

literals down to a smaller representation than what Babel normally does,
because <Code>styled-components</Code> template literals don't need to
be 100% spec compliant. see{' '}
<a href="#tagged-template-literals">Tagged Template Literals</a> for
Copy link
Member

Choose a reason for hiding this comment

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

This link will need to be swapped out with our Link component 😉 You can see some examples for this on other pages

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed @philpl 🎉

@@ -163,7 +164,7 @@ const BabelPlugin = () =>
literals down to a smaller representation than what Babel normally does,
because <Code>styled-components</Code> template literals don't need to
be 100% spec compliant. see{' '}
<a href="#tagged-template-literals">Tagged Template Literals</a> for
<Link href="#tagged-template-literals">Tagged Template Literals</Link> for
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for not being clear enough on this :( You'll have to pass the inline prop for it to show up with the correct styling: https://github.com/styled-components/styled-components-website/blob/master/components/api/helpers/with-theme.js#L52

Copy link
Member Author

Choose a reason for hiding this comment

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

Got it ! It does look better now :p

@kitten kitten merged commit 573efef into styled-components:master Jul 25, 2017
@kitten
Copy link
Member

kitten commented Jul 25, 2017

@SaraVieira Thanks a lot for putting in such an effort to finally bring this section to our site 🎉 😄 I'll try to find a workaround for the SSR problem and then deploy a new version

@SaraVieira
Copy link
Member Author

@philpl No problem man , I'm now working on adding eslint 😄

@mxstbr
Copy link
Member

mxstbr commented Jul 25, 2017

Yay this is awesome, thanks so much for the help @SaraVieira!

@kitten
Copy link
Member

kitten commented Jul 25, 2017

@SaraVieira @mxstbr Should be deployed and the SSR bug should also be fixed (vendored a modified next.js version f9a9320)

@mxstbr can you clear the cache so we get the website to be quick again asap please? :)

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.

None yet

4 participants