Merge pull request #227 from benmosher/resolvers-v2

prototype of clearer resolver interface

.travis.yml

@@ -6,7 +6,7 @@ node_js:
   - stable
 
 install:
-  - npm -g install npm@2
+  - npm -g install npm@3
   - npm install
   # install all resolver deps
   - "for resolver in ./resolvers/*; do cd $resolver && npm install && cd ../..; done"

CHANGELOG.md

@@ -3,9 +3,15 @@ All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com).
 
-## [Unreleased]
+## [1.4.0] - 2016-03-25
+### Added
+- Resolver plugin interface v2: more explicit response format that more clearly covers the found-but-core-module case, where there is no path.
+  Still backwards-compatible with the original version of the resolver spec.
+- [Resolver documentation](./resolvers/README.md)
+
 ### Changed
 - using `package.json/files` instead of `.npmignore` for package file inclusion ([#228], thanks [@mathieudutour])
+- using `es6-*` ponyfills instead of `babel-runtime`
 
 ## [1.3.0] - 2016-03-20
 Major perf improvements. Between parsing only once and ignoring gigantic, non-module `node_modules`,
@@ -137,7 +143,8 @@ for info on changes for earlier releases.
 [#119]: https://github.com/benmosher/eslint-plugin-import/issues/119
 [#89]: https://github.com/benmosher/eslint-plugin-import/issues/89
 
-[Unreleased]: https://github.com/benmosher/eslint-plugin-import/compare/v1.3.0...HEAD
+[Unreleased]: https://github.com/benmosher/eslint-plugin-import/compare/v1.4.0...HEAD
+[1.4.0]: https://github.com/benmosher/eslint-plugin-import/compare/v1.3.0...v1.4.0
 [1.3.0]: https://github.com/benmosher/eslint-plugin-import/compare/v1.2.0...v1.3.0
 [1.2.0]: https://github.com/benmosher/eslint-plugin-import/compare/v1.1.0...v1.2.0
 [1.1.0]: https://github.com/benmosher/eslint-plugin-import/compare/v1.0.4...v1.1.0

README.md

@@ -85,7 +85,7 @@ rules:
   # etc...
 ```
 
-# Resolver plugins
+# Resolvers
 
 With the advent of module bundlers and the current state of modules and module
 syntax specs, it's not always obvious where `import x from 'module'` should look
@@ -99,48 +99,10 @@ Node does not, such as loaders (`import 'file!./whatever'`) and a number of
 aliasing schemes, such as [`externals`]: mapping a module id to a global name at
 runtime (allowing some modules to be included more traditionally via script tags).
 
-In the interest of supporting both of these, v0.11 introduces resolver plugins.
-At the moment, these are modules exporting a single function:
-
-```js
-
-exports.resolveImport = function (source, file, config) {
-  // return source's absolute path given
-  // - file: absolute path of importing module
-  // - config: optional config provided for this resolver
-
-  // return `null` if source is a "core" module (i.e. "fs", "crypto") that
-  // can't be found on the filesystem
-}
-```
-
-The default `node` plugin that uses [`resolve`] is a handful of lines:
-
-```js
-var resolve = require('resolve')
-  , path = require('path')
-  , assign = require('object-assign')
-
-exports.resolveImport = function resolveImport(source, file, config) {
-  if (resolve.isCore(source)) return null
-
-  return resolve.sync(source, opts(path.dirname(file), config))
-}
-
-function opts(basedir, config) {
-  return assign( {}
-               , config
-               , { basedir: basedir }
-               )
-}
-```
-
-It essentially just uses the current file to get a reference base directory (`basedir`)
-and then passes through any explicit config from the `.eslintrc`; things like
-non-standard file extensions, module directories, etc.
+In the interest of supporting both of these, v0.11 introduces resolvers.
 
 Currently [Node] and [Webpack] resolution have been implemented, but the
-resolvers are just npm packages, so third party packages are supported (and encouraged!).
+resolvers are just npm packages, so [third party packages are supported](https://github.com/benmosher/eslint-plugin-import/wiki/Resolvers) (and encouraged!).
 
 Just install a resolver as `eslint-import-resolver-foo` and reference it as such:
 
@@ -157,6 +119,8 @@ settings:
     foo: { someConfigKey: value }
 ```
 
+If you are interesting in writing a resolver, see the [spec](./resolvers/README.md) for more details.
+
 [`resolve`]: https://www.npmjs.com/package/resolve
 [`externals`]: http://webpack.github.io/docs/library-and-externals.html
 
@@ -193,7 +157,7 @@ settings:
 
 #### `import/resolver`
 
-See [resolver plugins](#resolver-plugins).
+See [resolvers](#resolvers).
 
 #### `import/cache`
 

appveyor.yml

@@ -19,7 +19,7 @@ install:
   - ps: Install-Product node $env:nodejs_version
 
   # update npm (only needed for Node 0.10)
-  # - npm -g install npm@2
+  - npm -g install npm@3
   # - set PATH=%APPDATA%\npm;%PATH%
 
   # install modules

package.json

@@ -1,6 +1,6 @@
 {
   "name": "eslint-plugin-import",
-  "version": "1.3.0",
+  "version": "1.4.0",
   "description": "Import with sanity.",
   "main": "lib/index.js",
   "directories": {
@@ -16,6 +16,7 @@
     "cover": "gulp pretest && cross-env NODE_PATH=./lib istanbul cover --dir reports/coverage _mocha tests/lib/ -- --recursive -R progress",
     "posttest": "eslint ./src",
     "test": "cross-env NODE_PATH=./lib gulp test",
+    "test-all": "npm test && for resolver in ./resolvers/*; do cd $resolver && npm test && cd ../..; done",
     "ci-test": "eslint ./src && gulp pretest && cross-env NODE_PATH=./lib istanbul cover --report lcovonly --dir reports/coverage _mocha tests/lib/ -- --recursive --reporter dot",
     "debug": "cross-env NODE_PATH=./lib mocha debug --recursive --reporter dot tests/lib/",
     "prepublish": "gulp prepublish",
@@ -68,7 +69,7 @@
     "es6-map": "^0.1.3",
     "es6-set": "^0.1.4",
     "es6-symbol": "*",
-    "eslint-import-resolver-node": "^0.1.0",
+    "eslint-import-resolver-node": "^0.2.0",
     "object-assign": "^4.0.1"
   }
 }

resolvers/README.md

@@ -0,0 +1,84 @@
+# Resolver spec (v2)
+
+Resolvers must export two names:
+
+### `interfaceVersion => Number`
+
+This document currently describes version 2 of the resolver interface. As such, a resolver implementing this version should
+
+```js
+export const interfaceVersion = 2
+```
+or
+```js
+exports.interfaceVersion = 2
+```
+
+To the extent it is feasible, trailing versions of the resolvers will continue to be supported, at least until a major version bump on the plugin proper.
+
+Currently, version 1 is assumed if no `interfaceVersion` is available. (didn't think to define it until v2, heh. 😅)
+
+### `resolve(source, file, config) => { found: Boolean, path: String? }`
+
+Given:
+```js
+// /some/path/to/module.js
+import ... from './imported-file'
+```
+
+and
+
+```yaml
+# .eslintrc.yml
+---
+settings:
+  import/resolver:
+    my-cool-resolver: [some, stuff]
+    node: { paths: [a, b, c] }
+```
+
+#### Arguments
+
+The arguments provided will be:
+
+##### `source`
+the module identifier (`./imported-file`).
+
+##### `file`
+the absolute path to the file making the import (`/some/path/to/module.js`)
+
+##### `config`
+
+an object provided via the `import/resolver` setting.`my-cool-resolver` will get `["some", "stuff"]` as its `config`, while
+  `node` will get `{ "paths": ["a", "b", "c"] }` provided as `config`.
+
+#### Return value
+
+The first resolver to return `{found: true}` is considered the source of truth. The returned object has:
+
+- `found`: `true` if the `source` module can be resolved relative to `file`, else `false`
+- `path`: an absolute path `String` if the module can be located on the filesystem; else, `null`.
+
+An example of a `null` path is a Node core module, such as `fs` or `crypto`. These modules can always be resolved, but the path need not be provided as the plugin will not attempt to parse core modules at this time.
+
+If the resolver cannot resolve `source` relative to `file`, it should just return `{ found: false }`. No `path` key is needed in this case.
+
+## Example
+
+Here is most of the [Node resolver] at the time of this writing. It is just a wrapper around substack/Browserify's synchronous [`resolve`]:
+
+```js
+var resolve = require('resolve')
+
+exports.resolve = function (source, file, config) {
+  if (resolve.isCore(source)) return { found: true, path: null }
+  try {
+    return { found: true, path: resolve.sync(source, opts(file, config)) }
+  } catch (err) {
+    return { found: false }
+  }
+}
+```
+
+[Node resolver]: ./node/index.js
+[`resolve`]: https://www.npmjs.com/package/resolve

resolvers/node/.npmignore

@@ -0,0 +1 @@
+test/

resolvers/node/index.js

@@ -2,10 +2,15 @@ var resolve = require('resolve')
   , path = require('path')
   , assign = require('object-assign')
 
-exports.resolveImport = function resolveImport(source, file, config) {
-  if (resolve.isCore(source)) return null
+exports.interfaceVersion = 2
 
-  return resolve.sync(source, opts(file, config))
+exports.resolve = function (source, file, config) {
+  if (resolve.isCore(source)) return { found: true, path: null }
+  try {
+    return { found: true, path: resolve.sync(source, opts(file, config)) }
+  } catch (err) {
+    return { found: false }
+  }
 }
 
 function opts(file, config) {

resolvers/node/package.json

@@ -1,6 +1,6 @@
 {
   "name": "eslint-import-resolver-node",
-  "version": "0.1.1",
+  "version": "0.2.0",
   "description": "Node default behavior import resolution plugin for eslint-plugin-import.",
   "main": "index.js",
   "scripts": {
@@ -27,6 +27,9 @@
     "object-assign": "^4.0.1",
     "resolve": "^1.1.6"
   },
+  "peerDependencies": {
+    "eslint-plugin-import": ">=1.4.0"
+  },
   "devDependencies": {
     "chai": "^3.4.1",
     "mocha": "^2.3.4"

resolvers/node/test/paths.js

@@ -5,7 +5,8 @@ var node = require('../index.js')
 
 describe("paths", function () {
   it("handles base path relative to CWD", function () {
-    expect(node.resolveImport('../', './test/file.js'))
-      .to.equal(path.resolve(__dirname, '../index.js'))
+    expect(node.resolve('../', './test/file.js'))
+      .to.have.property('path')
+      .equal(path.resolve(__dirname, '../index.js'))
   })
 })

resolvers/webpack/index.js

@@ -8,6 +8,8 @@ var findRoot = require('find-root')
 
 var resolveAlias = require('./resolve-alias')
 
+exports.interfaceVersion = 2
+
 /**
  * Find the full path to 'source', given 'file' as a full reference path.
  *
@@ -18,15 +20,15 @@ var resolveAlias = require('./resolve-alias')
  * @return {string?} the resolved path to source, undefined if not resolved, or null
  *                   if resolved to a non-FS resource (i.e. script tag at page load)
  */
-exports.resolveImport = function resolveImport(source, file, settings) {
+exports.resolve = function (source, file, settings) {
 
   // strip loaders
   var finalBang = source.lastIndexOf('!')
   if (finalBang >= 0) {
     source = source.slice(finalBang + 1)
   }
 
-  if (resolve.isCore(source)) return null
+  if (resolve.isCore(source)) return { found: true, path: null }
 
   var configPath = get(settings, 'config', 'webpack.config.js')
     , webpackConfig
@@ -46,7 +48,7 @@ exports.resolveImport = function resolveImport(source, file, settings) {
   }
 
   // externals
-  if (findExternal(source, webpackConfig.externals)) return null
+  if (findExternal(source, webpackConfig.externals)) return { found: true, path: null }
 
   // replace alias if needed
   source = resolveAlias(source, get(webpackConfig, ['resolve', 'alias'], {}))
@@ -61,20 +63,24 @@ exports.resolveImport = function resolveImport(source, file, settings) {
   }
 
   // otherwise, resolve "normally"
-  return resolve.sync(source, {
-    basedir: path.dirname(file),
+  try {
+    return { found: true, path: resolve.sync(source, {
+      basedir: path.dirname(file),
 
-    // defined via http://webpack.github.io/docs/configuration.html#resolve-extensions
-    extensions: get(webpackConfig, ['resolve', 'extensions'])
-      || ['', '.webpack.js', '.web.js', '.js'],
+      // defined via http://webpack.github.io/docs/configuration.html#resolve-extensions
+      extensions: get(webpackConfig, ['resolve', 'extensions'])
+        || ['', '.webpack.js', '.web.js', '.js'],
 
-    // http://webpack.github.io/docs/configuration.html#resolve-modulesdirectories
-    moduleDirectory: get(webpackConfig, ['resolve', 'modulesDirectories'])
-      || ['web_modules', 'node_modules'],
+      // http://webpack.github.io/docs/configuration.html#resolve-modulesdirectories
+      moduleDirectory: get(webpackConfig, ['resolve', 'modulesDirectories'])
+        || ['web_modules', 'node_modules'],
 
-    paths: paths,
-    packageFilter: packageFilter.bind(null, webpackConfig),
-  })
+      paths: paths,
+      packageFilter: packageFilter.bind(null, webpackConfig),
+    }) }
+  } catch (err) {
+    return { found: false }
+  }
 }
 
 function findExternal(source, externals) {

resolvers/webpack/package.json

@@ -1,6 +1,6 @@
 {
   "name": "eslint-import-resolver-webpack",
-  "version": "0.1.5",
+  "version": "0.2.0",
   "description": "Resolve paths to dependencies, given a webpack.config.js. Plugin for eslint-plugin-import.",
   "main": "index.js",
   "scripts": {
@@ -30,6 +30,9 @@
     "lodash.get": "^3.7.0",
     "resolve": "^1.1.6"
   },
+  "peerDependencies": {
+    "eslint-plugin-import": ">=1.4.0"
+  },
   "devDependencies": {
     "chai": "^3.4.1",
     "mocha": "^2.3.3"

resolvers/webpack/test/alias.js

@@ -7,8 +7,13 @@ var webpack = require('../index')
 var file = path.join(__dirname, 'files', 'dummy.js')
 
 describe("resolve.alias", function () {
-  it("works", function () {
-    expect(webpack.resolveImport('foo', file)).to.exist
+  var resolved
+  before(function () { resolved = webpack.resolve('foo', file) })
+
+  it("is found", function () { expect(resolved).to.have.property('found', true) })
+
+  it("is correct", function () {
+    expect(resolved).to.have.property('path')
       .and.equal(path.join(__dirname, 'files', 'some', 'goofy', 'path', 'foo.js'))
   })
 })