Skip to content
This repository was archived by the owner on May 14, 2021. It is now read-only.

Add moduleName option #112

Merged
merged 10 commits into from
Sep 5, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
24 changes: 24 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,30 @@ if (condition) {

It may be that other tagged template literal styles are coincidentally supported, but no issues will be handled regarding indentation unless the above style was used.

### Usage with other libraries

Some other libraries also implement the `styled.x` pattern with tagged template literals. This processor will lint the CSS in those tagged template literals too.

If you want to avoid edge cases when you assing the default import to a variable not called "styled" set the `importName` option to the library name. (set to `styled-components` by default)

```js
import cool from 'other-library';

const Button = cool.button`
color: blue;
`
```

```json
{
"processors": [["stylelint-processor-styled-components", {
"importName": "other-library"
}]]
}
```

> **NOTE:** That double array is on purpose but only necessary if you set options, see the [processors configuration docs](https://stylelint.io/user-guide/configuration/#processors).

## License

Licensed under the MIT License, Copyright © 2017 Maximilian Stoiber. See [LICENSE.md](./LICENSE.md) for more information!
Expand Down
11 changes: 9 additions & 2 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,20 @@ const path = require('path')
const parse = require('./parsers/index')

const sourceMapsCorrections = {}
const DEFAULT_OPTIONS = {
importName: 'styled-components'
}

module.exports = (/* options */) => ({
module.exports = options => ({
// Get string for stylelint to lint
code(input, filepath) {
const absolutePath = path.resolve(process.cwd(), filepath)
sourceMapsCorrections[absolutePath] = {}
const { extractedCSS, sourceMap } = parse(input, absolutePath)
const { extractedCSS, sourceMap } = parse(
input,
absolutePath,
Object.assign({}, DEFAULT_OPTIONS, options)
)
// Save source location, merging existing corrections with current corrections
sourceMapsCorrections[absolutePath] = Object.assign(
sourceMapsCorrections[absolutePath],
Expand Down
8 changes: 4 additions & 4 deletions src/parsers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const getTTLContent = require('../utils/tagged-template-literal.js').getTaggedTe
const parseImports = require('../utils/parse').parseImports
const getSourceMap = require('../utils/parse').getSourceMap

const processStyledComponentsFile = (ast, absolutePath) => {
const processStyledComponentsFile = (ast, absolutePath, options) => {
const extractedCSS = []
let ignoreRuleComments = []
let importedNames = {
Expand All @@ -37,7 +37,7 @@ const processStyledComponentsFile = (ast, absolutePath) => {
}
})
}
if (isStyledImport(node)) {
if (isStyledImport(node, options.importName)) {
importedNames = parseImports(node)
return
}
Expand Down Expand Up @@ -75,7 +75,7 @@ const processStyledComponentsFile = (ast, absolutePath) => {
}
}

module.exports = (input, absolutePath) => {
module.exports = (input, absolutePath, options) => {
let ast = null
if (absolutePath.endsWith('.ts') || absolutePath.endsWith('.tsx')) {
// We import it dynamically in order to be able to not include typescript as a dependency
Expand All @@ -86,5 +86,5 @@ module.exports = (input, absolutePath) => {
} else {
ast = estreeParse(input)
}
return processStyledComponentsFile(ast, absolutePath)
return processStyledComponentsFile(ast, absolutePath, options)
}
4 changes: 2 additions & 2 deletions src/utils/styled.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ const isTaggedTemplateLiteral = require('./tagged-template-literal').isTaggedTem
/**
* Check if something is a styled-components import declaration
*/
const isStyledImport = node =>
node.type === 'ImportDeclaration' && node.source.value === 'styled-components'
const isStyledImport = (node, importName) =>
node.type === 'ImportDeclaration' && node.source.value === importName
Copy link
Member

Choose a reason for hiding this comment

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

Hmm in the README you're saying this depends on the name you import it with, but here we check the library imported, so you should probably update that in the README. Our code is actually agnostic to the variable names you assign them to, as far as I understand anyway. So therefore if you want to use it with another library you would have to specify that option, as we never check for variables called styled I believe.

In that case the argument here and the option name should probably be libraryName, yeah?

Copy link
Member Author

@mxstbr mxstbr Aug 28, 2017

Choose a reason for hiding this comment

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

Read the associated issue, I just phrased the README badly.

We default to styled for the assigned variable name of the default export, so it'll work if you do import styled from 'emotion', and we also check the imports so it works if you do import other from 'styled-components', but at the moment if you combine those two things and do import other from 'emotion' it'll break. That's what this PR fixes, basically.

Would appreciate PRs improving the README!

Maybe libraryName is the better name for the option (might as well also make it library?), I decided to go for importName because a) I wasn't sure if packageName or libraryName were clearer, and b) if you alias stuff with webpack or something it's not the library name but the name that it's imported from, so I just went with importName since it's unambiguous. (admittedly aliasing is a niche use case, but it'd be annoying to break users who do that)

Copy link
Member Author

Choose a reason for hiding this comment

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

I remembered why I called it importName added my reasoning in the above comment. I can see why importName could be confusing, since "name" doesn't unambigously point to either parts of the import statement.

Babel calls it importNode.source.value, so maybe importSourceName?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe I'm thinking this through too much 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Hmm okay, I think I get you now. My main points of misunderstanding was mostly from this line in the README:

If you want to avoid edge cases when you assing the default import to a variable not called "styled"

As I think that's super misleading as it actually has nothing to do with what you name your the variable you import it into (it would also work, as I know now you didn't have an understanding problem with, if you did import wabalubadubdub from 'styled-components).

And then I also had a problem because I never heard of the webpack aliasing thing because I never used it, when you talked about aliasing I thought you might've meant import { injectGlobal as some_alias } or stuff like that which again wouldn't have been relevant but I researched the webpack alias thing, and get you now!


/**
* Check if something is a styled shorthand call
Expand Down
6 changes: 6 additions & 0 deletions test/fixtures/options/import-name.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import emotion from 'emotion'

// ⚠️ EMPTY BLOCK ⚠️
const Button = emotion.div`

`
71 changes: 71 additions & 0 deletions test/options.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
const stylelint = require('stylelint')
const path = require('path')

const processor = path.join(__dirname, '../src/index.js')
const rules = {
'block-no-empty': true,
indentation: 2,
'rule-empty-line-before': [
'always-multi-line',
{
except: ['first-nested'],
ignore: ['after-comment']
}
],
'selector-type-no-unknown': true
}

describe('options', () => {
let fixture
let data

// NOTE beforeEach() runs _after_ the beforeAll() hooks of the describe() blocks, so `fixture`
// will have the right path
beforeEach(done => {
stylelint
.lint({
files: [fixture],
syntax: 'scss',
config: {
// Set importName option to "emotion"
processors: [[processor, { importName: 'emotion' }]],
rules
}
})
.then(result => {
data = result
done()
})
.catch(err => {
console.log(err)
data = err
done()
})
})

describe('importName', () => {
beforeAll(() => {
fixture = path.join(__dirname, './fixtures/options/import-name.js')
})

it('should have one result', () => {
expect(data.results.length).toEqual(1)
})

it('should use the right file', () => {
expect(data.results[0].source).toEqual(fixture)
})

it('should have errored', () => {
expect(data.results[0].errored).toEqual(true)
})

it('should have one warning (i.e. wrong lines of code)', () => {
expect(data.results[0].warnings.length).toEqual(1)
})

it('should have a block-no-empty as the first warning', () => {
expect(data.results[0].warnings[0].rule).toEqual('block-no-empty')
})
})
})