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

fix: make styled-jsx/babel plugin respect the source type #684

Merged
merged 3 commits into from
Jan 14, 2021
Merged

fix: make styled-jsx/babel plugin respect the source type #684

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4eb109d
Make the styled-jsx/babel Babel plugin respect the Babel source type.
jaydenseric Nov 19, 2020
0577121
chore: retrigger
Timer Jan 14, 2021
0fae5a3
Merge branch 'master' into jaydenseric/fix-680
Timer Jan 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
}
},
"dependencies": {
"@babel/helper-module-imports": "7.12.5",
"@babel/types": "7.8.3",
"babel-plugin-syntax-jsx": "6.18.0",
"convert-source-map": "1.7.0",
Expand Down
11 changes: 5 additions & 6 deletions src/_utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import path from 'path'
import { addDefault } from '@babel/helper-module-imports';
import * as t from '@babel/types'
import _hashString from 'string-hash'
import { SourceMapGenerator } from 'source-map'
Expand Down Expand Up @@ -621,14 +622,12 @@ export const booleanOption = opts => {
}

export const createReactComponentImportDeclaration = state => {
const styleModule =
addDefault(
state.file.path,
typeof state.opts.styleModule === 'string'
? state.opts.styleModule
: 'styled-jsx/style'

return t.importDeclaration(
[t.importDefaultSpecifier(t.identifier(STYLE_COMPONENT))],
t.stringLiteral(styleModule)
: 'styled-jsx/style',
{ nameHint: STYLE_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.

I'm not 100% sure what will happen if there is an existing variable in scope that has the same name as STYLE_COMPONENT (_JSXStyle), but I think the addDefault helper will modify the nameHint to avoid a collision. Ideally, no nameHint would be used and the automatic name of the injected default import would be used throughout the code instead of a hardcoded constant. This way a user could have a _JSXStyle variable name in the same scope, no problem.

Copy link
Member

Choose a reason for hiding this comment

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

All code branches that call this already check if STYLE_COMPONENT is in scope, so this should "just work".

Copy link
Collaborator

@giuseppeg giuseppeg Jan 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

There is a remote chance that _JSXStyle is in the source code but it is not styled-jsx's i.e. the user defined a binding with that name which will conflict with STYLE_COMPONENT.

If you want to fix this, you should probably generate a unique identifier from STYLE_COMPONENT, put it in state and pass it to helpers if necessary but I probably I wouldn't bother in this PR – it's been like that for 5-ish years

)
}

Expand Down
3 changes: 1 addition & 2 deletions src/babel-external.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,7 @@ export const visitor = {
!path.scope.hasBinding(STYLE_COMPONENT)
) {
state.hasInjectedJSXStyle = true
const importDeclaration = createReactComponentImportDeclaration(state)
path.scope.path.node.body.unshift(importDeclaration)
createReactComponentImportDeclaration(state)
}
})

Expand Down
5 changes: 2 additions & 3 deletions src/babel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ export default function({ types: t }) {
// Transpile external styles
path.traverse(externalStylesVisitor, state)
},
exit({ node, scope }, state) {
exit({ scope }, state) {
if (
!(
state.file.hasJSXStyle &&
Expand All @@ -306,8 +306,7 @@ export default function({ types: t }) {
}

state.hasInjectedJSXStyle = true
const importDeclaration = createReactComponentImportDeclaration(state)
node.body.unshift(importDeclaration)
createReactComponentImportDeclaration(state)
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/macro.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,7 @@ function styledJsxMacro({ references, state }) {
!path.scope.hasBinding(STYLE_COMPONENT)
) {
state.hasInjectedJSXStyle = true
const importDeclaration = createReactComponentImportDeclaration(state)
path.findParent(p => p.isProgram()).node.body.unshift(importDeclaration)
createReactComponentImportDeclaration(state)
}
})
})
Expand Down
5 changes: 3 additions & 2 deletions test/babel6/snapshots/attribute.js.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,10 @@ Generated by [AVA](https://ava.li).

> Snapshot 1

`var _this = this;␊
`import _JSXStyle from "styled-jsx/style";␊
␊
var _this = this;␊
␊
import _JSXStyle from "styled-jsx/style";␊
export default (() => {␊
const Element = 'div';␊
return <div className={"jsx-2886504620"}>␊
Expand Down
Binary file modified test/babel6/snapshots/attribute.js.snap
View file
Open in desktop
Binary file not shown.
4 changes: 2 additions & 2 deletions test/babel6/snapshots/external.js.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ Generated by [AVA](https://ava.li).
const baz = ['div{font-size:3em;}'];␊
␊
baz.__hash = '2141779268';␊
a.__hash = '262929833';␊
const a = [`div{font-size:${size}em;}`];␊
␊
a.__hash = '262929833';␊
export const uh = bar;␊
␊
export const foo = [`div.jsx-2299908427{color:${color};}`];␊
Expand Down Expand Up @@ -123,9 +123,9 @@ Generated by [AVA](https://ava.li).
const baz = new String('div{font-size:3em;}');␊
␊
baz.__hash = '2141779268';␊
a.__hash = '262929833';␊
const a = new String(`div{font-size:${size}em;}`);␊
␊
a.__hash = '262929833';␊
export const uh = bar;␊
␊
export const foo = new String(`div.jsx-2299908427{color:${color};}`);␊
Expand Down
Binary file modified test/babel6/snapshots/external.js.snap
View file
Open in desktop
Binary file not shown.
Binary file modified test/babel6/snapshots/index.js.snap
View file
Open in desktop
Binary file not shown.
Binary file modified test/babel6/snapshots/plugins.js.snap
View file
Open in desktop
Binary file not shown.
7 changes: 7 additions & 0 deletions test/fixtures/cjs-module.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use strict'

module.exports = () => (
<div>
<style jsx>{'div { color: red }'}</style>
</div>
)
7 changes: 7 additions & 0 deletions test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ test('works with stateless', async t => {
t.snapshot(code)
})

test('works with a CJS module', async t => {
const { code } = await transform('./fixtures/cjs-module.js', {
sourceType: 'script'
})
t.snapshot(code)
})

test('works with fragment', async t => {
const { code } = await transform('./fixtures/fragment.js')
t.snapshot(code)
Expand Down
38 changes: 19 additions & 19 deletions test/snapshots/external.js.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ Generated by [AVA](https://ava.li).
bar.__hash = "2141779268";␊
const baz = ["div{font-size:3em;}"];␊
baz.__hash = "2141779268";␊
a.__hash = "262929833";␊
const a = [`div{font-size:${size}em;}`];␊
a.__hash = "262929833";␊
export const uh = bar;␊
export const foo = [`div.jsx-2299908427{color:${color};}`];␊
foo.__hash = "2299908427";␊
Expand Down Expand Up @@ -49,6 +49,23 @@ Generated by [AVA](https://ava.li).
_defaultExport.__hash = "2292456818";␊
module.exports = _defaultExport;`

## Make sure that it works with the new automatic transform

> Snapshot 1

`import { jsx as _jsx } from "react/jsx-runtime";␊
import _JSXStyle from "styled-jsx/style";␊
const A = {␊
styles: /*#__PURE__*/_jsx(_JSXStyle, {␊
id: "2723508961",␊
children: "div.jsx-2723508961{color:green;}"␊
}),␊
className: "jsx-2723508961"␊
};␊
export default function IndexPage() {␊
return JSON.stringify(A);␊
}`

## Makes sure that style nodes are not re-used

> Snapshot 1
Expand Down Expand Up @@ -119,8 +136,8 @@ Generated by [AVA](https://ava.li).
bar.__hash = "2141779268";␊
const baz = new String("div{font-size:3em;}");␊
baz.__hash = "2141779268";␊
a.__hash = "262929833";␊
const a = new String(`div{font-size:${size}em;}`);␊
a.__hash = "262929833";␊
export const uh = bar;␊
export const foo = new String(`div.jsx-2299908427{color:${color};}`);␊
foo.__hash = "2299908427";␊
Expand Down Expand Up @@ -225,20 +242,3 @@ Generated by [AVA](https://ava.li).
<p className={`jsx-${styles.__hash}`}>test</p>␊
<_JSXStyle id={styles.__hash}>{styles}</_JSXStyle>␊
</div>);`

## Make sure that it works with the new automatic transform

> Snapshot 1

`import { jsx as _jsx } from "react/jsx-runtime";␊
import _JSXStyle from "styled-jsx/style";␊
const A = {␊
styles: /*#__PURE__*/_jsx(_JSXStyle, {␊
id: "2723508961",␊
children: "div.jsx-2723508961{color:green;}"␊
}),␊
className: "jsx-2723508961"␊
};␊
export default function IndexPage() {␊
return JSON.stringify(A);␊
}`
Binary file modified test/snapshots/external.js.snap
View file
Open in desktop
Binary file not shown.
14 changes: 14 additions & 0 deletions test/snapshots/index.js.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,20 @@ Generated by [AVA](https://ava.li).
<_JSXStyle id={"2529315885"}>{"span.jsx-2529315885{color:red;}"}</_JSXStyle>␊
</div>;`

## works with a CJS module

> Snapshot 1

`'use strict';␊
␊
var _JSXStyle = _interopRequireDefault(require("styled-jsx/style")).default;␊
␊
function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }␊
␊
module.exports = () => <div className={"jsx-2886504620"}>␊
<_JSXStyle id={"2886504620"}>{"div.jsx-2886504620{color:red;}"}</_JSXStyle>␊
</div>;`

## works with class

> Snapshot 1
Expand Down
Binary file modified test/snapshots/index.js.snap
View file
Open in desktop
Binary file not shown.
Binary file modified test/snapshots/plugins.js.snap
View file
Open in desktop
Binary file not shown.
38 changes: 3 additions & 35 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@
dependencies:
"@babel/types" "^7.12.1"

"@babel/helper-module-imports@^7.12.1":
"@babel/helper-module-imports@7.12.5", "@babel/helper-module-imports@^7.12.1":
version "7.12.5"
resolved "https://registry.yarnpkg.com/@babel/helper-module-imports/-/helper-module-imports-7.12.5.tgz#1bfc0229f794988f76ed0a4d4e90860850b54dfb"
integrity sha512-SR713Ogqg6++uexFRORf/+nPXMmWIn80TALu0uaFb+iQIUoR7bOC7zBWyzBs5b3tBBJXuyD0cRu1F15GyzjOWA==
Expand Down Expand Up @@ -3065,7 +3065,7 @@ debug@^4.0.1, debug@^4.1.0, debug@^4.1.1:
dependencies:
ms "^2.1.1"

debuglog@*, debuglog@^1.0.1:
debuglog@^1.0.1:
version "1.0.1"
resolved "https://registry.yarnpkg.com/debuglog/-/debuglog-1.0.1.tgz#aa24ffb9ac3df9a2351837cfb2d279360cd78492"
integrity sha1-qiT/uaw9+aI1GDfPstJ5NgzXhJI=
Expand Down Expand Up @@ -4718,7 +4718,7 @@ import-modules@^1.1.0:
resolved "https://registry.yarnpkg.com/import-modules/-/import-modules-1.1.0.tgz#748db79c5cc42bb9701efab424f894e72600e9dc"
integrity sha1-dI23nFzEK7lwHvq0JPiU5yYA6dw=

imurmurhash@*, imurmurhash@^0.1.4:
imurmurhash@^0.1.4:
version "0.1.4"
resolved "https://registry.yarnpkg.com/imurmurhash/-/imurmurhash-0.1.4.tgz#9218b9b2b928a238b13dc4fb6b6d576f231453ea"
integrity sha1-khi5srkoojixPcT7a21XbyMUU+o=
Expand Down Expand Up @@ -5645,11 +5645,6 @@ lockfile@^1.0.4:
dependencies:
signal-exit "^3.0.2"

lodash._baseindexof@*:
version "3.1.0"
resolved "https://registry.yarnpkg.com/lodash._baseindexof/-/lodash._baseindexof-3.1.0.tgz#fe52b53a1c6761e42618d654e4a25789ed61822c"
integrity sha1-/lK1OhxnYeQmGNZU5KJXie1hgiw=

lodash._baseuniq@~4.6.0:
version "4.6.0"
resolved "https://registry.yarnpkg.com/lodash._baseuniq/-/lodash._baseuniq-4.6.0.tgz#0ebb44e456814af7905c6212fa2c9b2d51b841e8"
Expand All @@ -5658,33 +5653,11 @@ lodash._baseuniq@~4.6.0:
lodash._createset "~4.0.0"
lodash._root "~3.0.0"

lodash._bindcallback@*:
version "3.0.1"
resolved "https://registry.yarnpkg.com/lodash._bindcallback/-/lodash._bindcallback-3.0.1.tgz#e531c27644cf8b57a99e17ed95b35c748789392e"
integrity sha1-5THCdkTPi1epnhftlbNcdIeJOS4=

lodash._cacheindexof@*:
version "3.0.2"
resolved "https://registry.yarnpkg.com/lodash._cacheindexof/-/lodash._cacheindexof-3.0.2.tgz#3dc69ac82498d2ee5e3ce56091bafd2adc7bde92"
integrity sha1-PcaayCSY0u5ePOVgkbr9Ktx73pI=

lodash._createcache@*:
version "3.1.2"
resolved "https://registry.yarnpkg.com/lodash._createcache/-/lodash._createcache-3.1.2.tgz#56d6a064017625e79ebca6b8018e17440bdcf093"
integrity sha1-VtagZAF2JeeevKa4AY4XRAvc8JM=
dependencies:
lodash._getnative "^3.0.0"

lodash._createset@~4.0.0:
version "4.0.3"
resolved "https://registry.yarnpkg.com/lodash._createset/-/lodash._createset-4.0.3.tgz#0f4659fbb09d75194fa9e2b88a6644d363c9fe26"
integrity sha1-D0ZZ+7CddRlPqeK4imZE02PJ/iY=

lodash._getnative@*, lodash._getnative@^3.0.0:
version "3.9.1"
resolved "https://registry.yarnpkg.com/lodash._getnative/-/lodash._getnative-3.9.1.tgz#570bc7dede46d61cdcde687d65d3eecbaa3aaff5"
integrity sha1-VwvH3t5G1hzc3mh9ZdPuy6o6r/U=

lodash._root@~3.0.0:
version "3.0.1"
resolved "https://registry.yarnpkg.com/lodash._root/-/lodash._root-3.0.1.tgz#fba1c4524c19ee9a5f8136b4609f017cf4ded692"
Expand Down Expand Up @@ -5785,11 +5758,6 @@ lodash.mergewith@^4.6.1:
resolved "https://registry.yarnpkg.com/lodash.mergewith/-/lodash.mergewith-4.6.2.tgz#617121f89ac55f59047c7aec1ccd6654c6590f55"
integrity sha512-GK3g5RPZWTRSeLSpgP8Xhra+pnjBC56q9FZYe1d5RN3TJ35dbkGy3YqBSMbyCrlbi+CM9Z3Jk5yTL7RCsqboyQ==

lodash.restparam@*:
version "3.6.1"
resolved "https://registry.yarnpkg.com/lodash.restparam/-/lodash.restparam-3.6.1.tgz#936a4e309ef330a7645ed4145986c85ae5b20805"
integrity sha1-k2pOMJ7zMKdkXtQUWYbIWuWyCAU=

lodash.snakecase@^4.0.1:
version "4.1.1"
resolved "https://registry.yarnpkg.com/lodash.snakecase/-/lodash.snakecase-4.1.1.tgz#39d714a35357147837aefd64b5dcbb16becd8f8d"
Expand Down