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

Feature/dynamic elements #462

Merged
merged 5 commits into from Jul 18, 2018
Merged

Conversation

twltwl
Copy link
Contributor

@twltwl twltwl commented Jul 11, 2018

Related issue #147

Based on comment by @giuseppeg

Automaticly scopes elements if possible ( works if declared inside of component )

for other cases a styled-jsx prop can be added

// works
const Cmp = () => {
  const Element = 'div';
  return (<Element className="foo">
    <style jsx>{`.foo{...}`}</style>
  </Element>)
}

// wont work
const Element = 'div';
const Cmp = () => (<Element className="foo">
  <style jsx>{`.foo{...}`}</style>
</Element>)

// works
const Element = 'div';
const Cmp = () => (<Element className="foo" styled-jsx>
  <style jsx>{`.foo{...}`}</style>
</Element>)

@twltwl twltwl requested a review from giuseppeg as a code owner July 11, 2018 08:47
Copy link
Collaborator

@giuseppeg giuseppeg left a comment

Choose a reason for hiding this comment

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

thank you this looks great! You will have to update the snapshots with ava -u

src/babel.js Outdated
Object.values(path.scope.bindings).some(binding =>
binding.referencePaths.some(r => r === tag)
) ||
(attributes &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we remove this feature for now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, if we want to add it at a later point let me know

@@ -34,6 +36,9 @@ export default () => (
<div {...props} {...rest} />
<div {...props} data-foo {...rest} />
<div {...props} className={'test'} data-foo {...rest} />
<Element styled-jsx />
Copy link
Collaborator

Choose a reason for hiding this comment

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

I won't want to include this feature here for now, let's fix one thing at the time

`}</style>
</Element>
)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

awesome can you add a few more tests cases? i.e. a few more combinations to where Element is defined outside of the component, and maybe when it is lowercase too (any edge case you can think of). Also can you add a class component too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe Im missing something, but I dont think it will work if Element is defined outside of the component. ( Thats why I added the styled-jsx option )
I´ll add the testcases

Copy link
Collaborator

Choose a reason for hiding this comment

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

correct, it won't work (it won't add the className) and we want a snapshot of that! ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Tommy *twl* Ljungberg added 2 commits July 11, 2018 17:24
Removed support for styled-jsx attribute matching
@giuseppeg giuseppeg added this to the v3 milestone Jul 18, 2018
@giuseppeg
Copy link
Collaborator

@twltwl this branch is good to merge, can you just remove node 6 from travis https://github.com/zeit/styled-jsx/blob/a55c821a998b9a7210c03a30c4ae33de7ec94eb6/.travis.yml#L3 ? and maybe add a new line for node 10?

@giuseppeg giuseppeg merged commit 8d9374a into vercel:next Jul 18, 2018
@giuseppeg
Copy link
Collaborator

@twltwl thanks so much for this great contribution! 🙌

@iamawebgeek
Copy link

@giuseppeg I cannot use this feature for some reason. Babel is not stripping out the styled-jsx attribute and adding a scoped className. How can I figure out what is the problem?

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

3 participants