Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: import-js/eslint-plugin-import
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: v1.1.0
Choose a base ref
...
head repository: import-js/eslint-plugin-import
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: v1.2.0
Choose a head ref
  • 11 commits
  • 7 files changed
  • 2 contributors

Commits on Mar 15, 2016

  1. docs: anchor fix

    benmosher committed Mar 15, 2016
    Copy the full SHA
    0942a7b View commit details
  2. Verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
    Copy the full SHA
    f3cb7ab View commit details

Commits on Mar 17, 2016

  1. Cache fileExists in core/resolve

    In a project where I have import/no-unresolved enabled, I get the
    following results with ESLint's TIMING=1 environment variable enabled:
    
    ```
    Rule                          | Time (ms) | Relative
    :-----------------------------|----------:|--------:
    import/no-unresolved          | 18563.065 |    40.6%
    react/sort-comp               |  1621.646 |     3.5%
    no-irregular-whitespace       |  1386.958 |     3.0%
    react/prop-types              |  1197.552 |     2.6%
    no-implied-eval               |   829.967 |     1.8%
    react/jsx-uses-vars           |   765.702 |     1.7%
    keyword-spacing               |   765.665 |     1.7%
    no-eval                       |   578.730 |     1.3%
    semi-spacing                  |   549.684 |     1.2%
    no-whitespace-before-property |   477.285 |     1.0%
    ```
    
    To speed up this outlier, I decided to cache the results of file
    existence checks in a Map. Adding this cache brings this time down
    from ~18s to ~13s:
    
    ```
    Rule                    | Time (ms) | Relative
    :-----------------------|----------:|--------:
    import/no-unresolved    | 13199.978 |    32.5%
    react/sort-comp         |  1793.781 |     4.4%
    react/prop-types        |  1400.975 |     3.4%
    no-irregular-whitespace |  1315.485 |     3.2%
    react/jsx-uses-vars     |   794.097 |     2.0%
    no-implied-eval         |   765.593 |     1.9%
    keyword-spacing         |   752.945 |     1.9%
    no-eval                 |   592.048 |     1.5%
    semi-spacing            |   532.061 |     1.3%
    no-control-regex        |   479.856 |     1.2%
    ```
    
    Addresses #187
    lencioni committed Mar 17, 2016
    Copy the full SHA
    5df6c55 View commit details

Commits on Mar 18, 2016

  1. Copy the full SHA
    137955b View commit details
  2. Copy the full SHA
    06e6244 View commit details
  3. Copy the full SHA
    dc1d4a4 View commit details
  4. import/cache: docs

    benmosher committed Mar 18, 2016
    Copy the full SHA
    753e25f View commit details
  5. Copy the full SHA
    7c63475 View commit details
  6. realized that resolving a file implies it exists, regardless of casing.

    only need to cache case-sensitivity.
    benmosher committed Mar 18, 2016
    Copy the full SHA
    a44774a View commit details

Commits on Mar 19, 2016

  1. Copy the full SHA
    c35b477 View commit details
  2. 1.2.0

    benmosher committed Mar 19, 2016
    Copy the full SHA
    9376fa4 View commit details
Showing with 181 additions and 23 deletions.
  1. +32 −1 README.md
  2. +1 −1 docs/rules/no-unresolved.md
  3. +1 −1 package.json
  4. +0 −1 src/core/getExports.js
  5. +67 −18 src/core/resolve.js
  6. 0 tests/files/CaseyKasem.js
  7. +80 −1 tests/src/core/resolve.js
33 changes: 32 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -171,11 +171,13 @@ You may set the following settings in your `.eslintrc`:

A list of regex strings that, if matched by a path, will
not report the matching module if no `export`s are found.
In practice, this means rules other than `no-unresolved` will not report on any
In practice, this means rules other than [`no-unresolved`](./docs/rules/no-unresolved.md#ignore) will not report on any
`import`s with (absolute) paths matching this pattern, _unless_ `export`s were
found when parsing. This allows you to ignore `node_modules` but still properly
lint packages that define a [`jsnext:main`] in `package.json` (Redux, D3's v4 packages, etc.).

`no-unresolved` has its own [`ignore`](./docs/rules/no-unresolved.md#ignore) setting.

**Note**: setting this explicitly will replace the default of `node_modules`, so you
may need to include it in your own list if you still want to ignore it. Example:

@@ -193,6 +195,35 @@ settings:

See [resolver plugins](#resolver-plugins).

#### `import/cache`

Settings for cache behavior. Memoization is used at various levels to avoid the copious amount of `fs.statSync`/module parse calls required to correctly report errors.

For normal `eslint` console runs, the cache lifetime is irrelevant, as we can strongly assume that files should not be changing during the lifetime of the linter process (and thus, the cache in memory)

For long-lasting processes, like [`eslint_d`] or [`eslint-loader`], however, it's important that there be some notion of staleness.

If you never use [`eslint_d`] or [`eslint-loader`], you may set the cache lifetime to `Infinity` and everything should be fine:

```yaml
# .eslintrc.yml
settings:
import/cache:
lifetime: ∞ # or Infinity
```

Otherwise, set some integer, and cache entries will be evicted after that many seconds have elapsed:

```yaml
# .eslintrc.yml
settings:
import/cache:
lifetime: 5 # 30 is the default
```

[`eslint_d`]: https://www.npmjs.com/package/eslint_d
[`eslint-loader`]: https://www.npmjs.com/package/eslint-loader

## SublimeLinter-eslint

SublimeLinter-eslint introduced a change to support `.eslintignore` files
2 changes: 1 addition & 1 deletion docs/rules/no-unresolved.md
Original file line number Diff line number Diff line change
@@ -80,4 +80,4 @@ a lot of false positive reports of missing dependencies.
- [Webpack resolver](https://npmjs.com/package/eslint-import-resolver-webpack)
- [`import/ignore`] global setting

[`import/ignore`]: ../../README.md#import/ignore
[`import/ignore`]: ../../README.md#importignore
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "eslint-plugin-import",
"version": "1.1.0",
"version": "1.2.0",
"description": "Import with sanity.",
"main": "lib/index.js",
"directories": {
1 change: 0 additions & 1 deletion src/core/getExports.js
Original file line number Diff line number Diff line change
@@ -344,4 +344,3 @@ function hashObject(object) {
settingsShasum.update(JSON.stringify(object))
return settingsShasum.digest('hex')
}
``
85 changes: 67 additions & 18 deletions src/core/resolve.js
Original file line number Diff line number Diff line change
@@ -1,37 +1,75 @@
import fs from 'fs'
import { dirname, basename, join } from 'path'

const CASE_INSENSITIVE = fs.existsSync(join(__dirname, 'reSOLVE.js'))
export const CASE_INSENSITIVE = fs.existsSync(join(__dirname, 'reSOLVE.js'))

// http://stackoverflow.com/a/27382838
function fileExistsWithCaseSync(filepath) {
var dir = dirname(filepath)
if (dir === '/' || dir === '.' || /^[A-Z]:\\$/i.test(dir)) return true
var filenames = fs.readdirSync(dir)
if (filenames.indexOf(basename(filepath)) === -1) {
return false
const fileExistsCache = new Map()

function cachePath(cacheKey, result) {
fileExistsCache.set(cacheKey, { result, lastSeen: Date.now() })
}

function checkCache(cacheKey, { lifetime }) {
if (fileExistsCache.has(cacheKey)) {
const { result, lastSeen } = fileExistsCache.get(cacheKey)
// check fresness
if (Date.now() - lastSeen < (lifetime * 1000)) return result
}
return fileExistsWithCaseSync(dir)
// cache miss
return undefined
}

function fileExists(filepath) {
if (CASE_INSENSITIVE) {
// short-circuit if path doesn't exist, ignoring case
return !(!fs.existsSync(filepath) || !fileExistsWithCaseSync(filepath))
// http://stackoverflow.com/a/27382838
function fileExistsWithCaseSync(filepath, cacheSettings) {
const dir = dirname(filepath)

let result = checkCache(filepath, cacheSettings)
if (result != null) return result

// base case
if (dir === '/' || dir === '.' || /^[A-Z]:\\$/i.test(dir)) {
result = true
} else {
return fs.existsSync(filepath)
const filenames = fs.readdirSync(dir)
if (filenames.indexOf(basename(filepath)) === -1) {
result = false
} else {
result = fileExistsWithCaseSync(dir, cacheSettings)
}
}
cachePath(filepath, result)
return result
}

export function relative(modulePath, sourceFile, settings) {

const sourceDir = dirname(sourceFile)
, cacheKey = sourceDir + hashObject(settings) + modulePath

const cacheSettings = Object.assign({
lifetime: 30, // seconds
}, settings['import/cache'])

// parse infinity
if (cacheSettings.lifetime === '∞' || cacheSettings.lifetime === 'Infinity') {
cacheSettings.lifetime = Infinity
}

const cachedPath = checkCache(cacheKey, cacheSettings)
if (cachedPath !== undefined) return cachedPath

function cache(path) {
cachePath(cacheKey, path)
return path
}

function withResolver(resolver, config) {
// resolve just returns the core module id, which won't appear to exist
try {
const filePath = resolver.resolveImport(modulePath, sourceFile, config)
if (filePath === null) return null
if (filePath == null) return filePath

if (filePath === undefined || !fileExists(filePath)) return undefined
// resolvers imply file existence, this double-check just ensures the case matches
if (CASE_INSENSITIVE && !fileExistsWithCaseSync(filePath, cacheSettings)) return undefined

return filePath
} catch (err) {
@@ -48,9 +86,12 @@ export function relative(modulePath, sourceFile, settings) {
const resolver = require(`eslint-import-resolver-${name}`)

let fullPath = withResolver(resolver, config)
if (fullPath !== undefined) return fullPath
if (fullPath !== undefined) {
return cache(fullPath)
}
}

return cache(undefined)
}

function resolverReducer(resolvers, map) {
@@ -89,3 +130,11 @@ export default function resolve(p, context) {
)
}
resolve.relative = relative


import { createHash } from 'crypto'
function hashObject(object) {
const settingsShasum = createHash('sha1')
settingsShasum.update(JSON.stringify(object))
return settingsShasum.digest('hex')
}
Empty file added tests/files/CaseyKasem.js
Empty file.
81 changes: 80 additions & 1 deletion tests/src/core/resolve.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { expect } from 'chai'
import resolve from 'core/resolve'

import resolve, { CASE_INSENSITIVE } from 'core/resolve'

import * as fs from 'fs'
import * as utils from '../utils'

describe('resolve', function () {
@@ -24,4 +26,81 @@ describe('resolve', function () {

expect(file, 'path to ./jsx/MyUncoolComponent').to.be.undefined
})

describe('case cache correctness', function () {
const context = utils.testContext({
'import/cache': { 'lifetime': 1 },
})

const pairs = [
['./CaseyKasem.js', './CASEYKASEM.js'],
]

pairs.forEach(([original, changed]) => {
describe(`${original} => ${changed}`, function () {

before('sanity check', function () {
expect(resolve(original, context)).to.exist
expect(resolve(changed, context)).not.to.exist
})

before('rename', function (done) {
fs.rename(
utils.testFilePath(original),
utils.testFilePath(changed),
done)
})

before('verify rename', (done) =>
fs.exists(
utils.testFilePath(changed),
exists => done(exists ? null : new Error('new file does not exist'))))

// these tests fail on a case-sensitive file system
// because nonexistent files aren't cached
if (CASE_INSENSITIVE) {
it('gets cached values within cache lifetime', function () {
// get cached values initially
expect(resolve(original, context)).to.exist
expect(resolve(changed, context)).not.to.exist
})

// special behavior for infinity
describe('infinite cache', function () {
this.timeout(1200)
before((done) => setTimeout(done, 1100))

const lifetimes = [ '∞', 'Infinity' ]
lifetimes.forEach(inf => {
const infiniteContext = utils.testContext({
'import/cache': { 'lifetime': inf },
})

it(`lifetime: ${inf} still gets cached values after ~1s`, function () {
expect(resolve(original, infiniteContext)).to.exist
expect(resolve(changed, infiniteContext)).not.to.exist
})
})
})
}

describe('finite cache', function () {
this.timeout(1200)
before((done) => setTimeout(done, 1000))
it('gets correct values after cache lifetime', function () {
expect(resolve(original, context)).not.to.exist
expect(resolve(changed, context)).to.exist
})
})

after('restore original case', function (done) {
fs.rename(
utils.testFilePath(changed),
utils.testFilePath(original),
done)
})
})
})
})

})