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

webpack resolver: cache instance(s) of resolve function #1091

Merged
merged 4 commits into from
May 3, 2018
Merged

webpack resolver: cache instance(s) of resolve function #1091

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
198a9a8
cache instance(s) of resolve function
benmosher Apr 30, 2018
16a4861
better lodash requires
benmosher May 1, 2018
ee71e49
properly report resolver test coverage
benmosher May 1, 2018
4a3d5ad
went the wrong way on the isAbsolute change before
benmosher May 1, 2018
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
25 changes: 23 additions & 2 deletions resolvers/webpack/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
var findRoot = require('find-root')
, path = require('path')
, get = require('lodash.get')
, _ = require('lodash')
Copy link
Member

Choose a reason for hiding this comment

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

i believe this should actually be get = require('lodash/get') for maximum tree-shake-ability, and for future compat with lodash 5.

Copy link
Member Author

Choose a reason for hiding this comment

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

ooo cool, thanks. I have yet to get familiar with digging things directly out of lodash

, find = require('array-find')
, interpret = require('interpret')
// not available on 0.10.x
Expand All @@ -11,6 +11,8 @@ var findRoot = require('find-root')
, semver = require('semver')
, has = require('has')

var get = _.get

var log = require('debug')('eslint-plugin-import:resolver:webpack')

exports.interfaceVersion = 2
Expand Down Expand Up @@ -105,7 +107,8 @@ exports.resolve = function (source, file, settings) {
}

// otherwise, resolve "normally"
var resolveSync = createResolveSync(configPath, webpackConfig)
var resolveSync = getResolveSync(configPath, webpackConfig)

try {
return { found: true, path: resolveSync(path.dirname(file), source) }
} catch (err) {
Expand All @@ -114,6 +117,24 @@ exports.resolve = function (source, file, settings) {
}
}

var MAX_CACHE = 10
var _cache = []
function getResolveSync(configPath, webpackConfig) {
var cacheKey = { configPath: configPath, webpackConfig: webpackConfig }
var cached = find(_cache, function (entry) { return _.isEqual(entry.key, cacheKey) })
Copy link
Member

Choose a reason for hiding this comment

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

is there a reason to use an array here, rather than a Set, given that eslint only works on node 4+?

Copy link
Member Author

Choose a reason for hiding this comment

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

you could arguably still use the latest webpack resolver with much older versions of ESLint and this plugin (at work I only just recently upgraded us from ESLint 2 and plugin v1).

I don't think it is critical to maintain the ES5, at this point, but I also didn't want to cross that bridge today if there wasn't a compelling reason. was easy enough to write this PR in ES5.

if (!cached) {
cached = {
key: cacheKey,
value: createResolveSync(configPath, webpackConfig)
}
// put in front and pop last item
if (_cache.unshift(cached) > MAX_CACHE) {
_cache.pop()
}
}
return cached.value
}

function createResolveSync(configPath, webpackConfig) {
var webpackRequire
, basedir = null
Expand Down
2 changes: 1 addition & 1 deletion resolvers/webpack/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
"has": "^1.0.1",
"interpret": "^1.0.0",
"is-absolute": "^0.2.3",
"lodash.get": "^4.4.2",
"lodash": "^4.17.4",
"node-libs-browser": "^1.0.0 || ^2.0.0",
"resolve": "^1.4.0",
"semver": "^5.3.0"
Expand Down