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.

Sign up for GitHub

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

Jump to bottom

Feature/dynamic elements #462

Merged
merged 5 commits into from Jul 18, 2018
Merged

Feature/dynamic elements #462

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2966b39
Add support for dynamic elements
Jul 11, 2018
fb79022
Automaticly scope dynamic elements if possible
Jul 11, 2018
ca63098
Added testcases for dynamic elements
Jul 11, 2018
edd4a6f
update snapshots
Jul 11, 2018
942107b
update node version in travis
Jul 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/babel.js
View file
Open in desktop
Expand Up @@ -26,6 +26,7 @@ export default function({ types: t }) {
JSXOpeningElement(path, state) {
const el = path.node
const { name } = el.name || {}
const { attributes } = el || {}

if (!state.hasJSXStyle) {
return
Expand All @@ -40,11 +41,21 @@ export default function({ types: t }) {
state.ignoreClosing = 0
}

const tag = path.get('name')

if (
name &&
name !== 'style' &&
name !== STYLE_COMPONENT &&
name.charAt(0) !== name.charAt(0).toUpperCase()
(name.charAt(0) !== name.charAt(0).toUpperCase() ||
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

Object.values(attributes).some(
({ name, type }) =>
type === 'JSXAttribute' && name.name === 'styled-jsx'
)))
) {
if (state.className) {
addClassName(path, state.className)
Expand Down
5 changes: 5 additions & 0 deletions test/__snapshots__/attribute.js.snap
View file
Open in desktop
Expand Up @@ -100,6 +100,8 @@ exports[`rewrites className 1`] = `
"var _this = this;

import _JSXStyle from \\"styled-jsx/style\\";
const Element = 'div';

export default (() => <div className={\\"jsx-2886504620\\"}>
<div {...test.test} className={\\"jsx-2886504620\\" + \\" \\" + (test.test.className != null && test.test.className || \\"test\\")} />
<div {...test.test.test} className={\\"jsx-2886504620\\" + \\" \\" + (test.test.test.className != null && test.test.test.className || \\"test\\")} />
Expand Down Expand Up @@ -135,6 +137,9 @@ export default (() => <div className={\\"jsx-2886504620\\"}>
<div {...props} {...rest} className={\\"jsx-2886504620\\" + \\" \\" + (rest.className != null && rest.className || props.className != null && props.className || \\"\\")} />
<div {...props} data-foo {...rest} className={\\"jsx-2886504620\\" + \\" \\" + (rest.className != null && rest.className || props.className != null && props.className || \\"\\")} />
<div {...props} data-foo {...rest} className={\\"jsx-2886504620\\" + \\" \\" + (rest.className != null && rest.className || 'test')} />
<Element styled-jsx className={\\"jsx-2886504620\\"} />
<Element styled-jsx className={\\"jsx-2886504620\\" + \\" \\" + \\"test\\"} />
<Element styled-jsx {...props} className={\\"jsx-2886504620\\" + \\" \\" + (props.className != null && props.className || \\"\\")} />
<_JSXStyle styleId={\\"2886504620\\"} css={\\"div.jsx-2886504620{color:red;}\\"} />
</div>);"
`;
12 changes: 12 additions & 0 deletions test/__snapshots__/index.js.snap
View file
Open in desktop
Expand Up @@ -103,6 +103,18 @@ class Test extends React.Component {
}"
`;

exports[`works with dynamic element 1`] = `
"import _JSXStyle from \\"styled-jsx/style\\";
export default (({ level = 1 }) => {
const Element = \`h\${level}\`;

return <Element className={\\"jsx-1253978709\\" + \\" \\" + \\"root\\"}>
<p className={\\"jsx-1253978709\\"}>dynamic element</p>
<_JSXStyle styleId={\\"1253978709\\"} css={\\".root.jsx-1253978709{background:red;}\\"} />
</Element>;
});"
`;

exports[`works with expressions in template literals 1`] = `
"import _JSXStyle from 'styled-jsx/style';
const darken = c => c;
Expand Down
5 changes: 5 additions & 0 deletions test/fixtures/attribute-generation-classname-rewriting.js
View file
Open in desktop
@@ -1,3 +1,5 @@
const Element = 'div'

export default () => (
<div>
<div className="test" {...test.test} />
Expand Down Expand Up @@ -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

<Element styled-jsx className="test" />
<Element styled-jsx {...props} />
<style jsx>{'div { color: red }'}</style>
</div>
)
14 changes: 14 additions & 0 deletions test/fixtures/dynamic-element.js
View file
Open in desktop
@@ -0,0 +1,14 @@
export default ({ level = 1 }) => {
const Element = `h${level}`

return (
<Element className="root">
<p>dynamic element</p>
<style jsx>{`
.root {
background: red;
}
`}</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.

👍

5 changes: 5 additions & 0 deletions test/index.js
View file
Open in desktop
Expand Up @@ -82,6 +82,11 @@ test('works with css tagged template literals in the same file', async t => {
t.snapshot(code)
})

test('works with dynamic element', async t => {
const { code } = await transform('./fixtures/dynamic-element.js')
t.snapshot(code)
})

test('does not transpile nested style tags', async t => {
const { message } = await t.throws(
transform('./fixtures/nested-style-tags.js')
Expand Down