-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Configurable imports #290
Configurable imports #290
Conversation
return { | ||
name: 'emotion', // not required | ||
inherits: require('babel-plugin-syntax-jsx'), | ||
visitor: { | ||
ImportDeclaration: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you move this below Program
?
export default function(babel) { | ||
const { types: t } = babel | ||
|
||
let importedNames = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The importedNames
arg values come from state.opts
. We might also want to set the imported names to state.importedNames
return { | ||
name: 'emotion', // not required | ||
inherits: require('babel-plugin-syntax-jsx'), | ||
visitor: { | ||
ImportDeclaration: { | ||
enter({ node }) { | ||
if (node.source.value === 'emotion') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be node.source.value.indexOf('emotion') !== -1
so it includes emotion, react-emotion, preact-emotion and any other runtimes people create.
74d7c5b
to
697b00f
Compare
Codecov Report
@@ Coverage Diff @@
## master #290 +/- ##
==========================================
+ Coverage 90.37% 90.45% +0.07%
==========================================
Files 21 21
Lines 966 974 +8
Branches 260 261 +1
==========================================
+ Hits 873 881 +8
Misses 75 75
Partials 18 18
Continue to review full report at Codecov.
|
} | ||
|
||
const defaultImportedNames = { | ||
default: 'styled', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change this from default
to styled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good with this as long as it's explicitly stated in the docs that it only works for ESM imports and all imports are supported
// Is default import | ||
names.default = singleImport.local.name | ||
} | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be done with state.file.metadata.modules.imports
in the Program
enter visitor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion. Was able to remove parseImports
and the ImportDeclaration
visitor.
docs/configurable-imports.md
Outdated
@@ -0,0 +1,101 @@ | |||
# Configurable Imports | |||
|
|||
If you are using ES Module imports (`import styled from 'emotion'`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
styled
doesn't exist in emotion. It's only in react-emotion and preact-emotion.
baf8694
to
6a4225e
Compare
docs/configurable-imports.md
Outdated
|
||
```js | ||
import _JSXStyle from "styled-jsx/style"; | ||
import styled, { css } from "react-emotion"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this say css as cows
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
Basically this is a POC that proves that this works without extra configuration. Breaks no existing tests. Only works for `styled` calls. ```js import lol from 'emotion'; lol`color:blue`; ```
Set importedNames on state and merge with babel opts.
* Remove ImportDeclaration visitor * Remove parseImports * Move import naming logic to `Program`
bba5b4c
to
7ec2f4d
Compare
Built on top of lint-fix changes (#289) -- compare between these two branchesWhat:
Solves #267 by implementing a solution that will use the default import name instead of sticking only to
styled
.in short, this works:
Why: See #267
How:
Modified the visitor to also visit ImportDeclarations. Then cache the values of any
import whatever from 'emotion'
in an object for later use when looking forstyled
calls.Checklist:
These need work. This is just a POC. There definitely needs to be more tests.