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.

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

Issue2024 #3

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
10 changes: 5 additions & 5 deletions .github/workflows/node-4+.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
with:
versionsAsRoot: true
type: majors
preset: '>=4'
preset: '>= 6' # preset: '>=4' # see https://github.com/benmosher/eslint-plugin-import/issues/2053

latest:
needs: [matrix]
Expand Down Expand Up @@ -66,18 +66,18 @@ jobs:

steps:
- uses: actions/checkout@v2
- uses: ljharb/actions/node/run@main
- uses: ljharb/actions/node/install@main
continue-on-error: ${{ matrix.eslint == 4 && matrix.node-version == 4 }}
name: 'npm install && npm run tests-only'
name: 'nvm install ${{ matrix.node-version }} && npm install, with eslint ${{ matrix.eslint }}'
env:
ESLINT_VERSION: ${{ matrix.eslint }}
TRAVIS_NODE_VERSION: ${{ matrix.node-version }}
with:
node-version: ${{ matrix.node-version }}
after_install: npm run copy-metafiles && ./tests/dep-time-travel.sh
command: 'tests-only'
after_success: 'npm run coveralls'
skip-ls-check: true
- run: npm run tests-only
- run: npm run coveralls

node:
name: 'node 4+'
Expand Down
13 changes: 7 additions & 6 deletions .github/workflows/node-pretest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,21 @@ jobs:

# steps:
# - uses: actions/checkout@v2
# - uses: ljharb/actions/node/run@main
# name: 'npm install && npm run pretest'
# - uses: ljharb/actions/node/install@main
# name: 'nvm install lts/* && npm install'
# with:
# node-version: 'lts/*'
# command: 'pretest'
# skip-ls-check: true
# - run: npm run pretest

posttest:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2
- uses: ljharb/actions/node/run@main
name: 'npm install && npm run posttest'
- uses: ljharb/actions/node/install@main
name: 'nvm install lts/* && npm install'
with:
node-version: 'lts/*'
command: 'posttest'
skip-ls-check: true
- run: npm run posttest
15 changes: 9 additions & 6 deletions .github/workflows/packages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
id: set-matrix
with:
type: 'majors'
preset: '>=4'
preset: '>= 6' # preset: '>=4' # see https://github.com/benmosher/eslint-plugin-import/issues/2053
versionsAsRoot: true

tests:
Expand All @@ -22,6 +22,7 @@ jobs:
runs-on: ubuntu-latest

strategy:
fail-fast: false
matrix:
node-version: ${{ fromJson(needs.matrix.outputs.latest) }}
package:
Expand All @@ -32,14 +33,16 @@ jobs:

steps:
- uses: actions/checkout@v2
- uses: ljharb/actions/node/run@main
name: 'npm install && npm run tests-only'
- uses: ljharb/actions/node/install@main
name: 'nvm install ${{ matrix.node-version }} && npm install'
env:
ESLINT_VERSION: ${{ matrix.eslint }}
TRAVIS_NODE_VERSION: ${{ matrix.node-version }}
with:
node-version: ${{ matrix.node-version }}
after_install: npm run copy-metafiles && cd ${{ matrix.package }} && npm install
command: 'tests-only'
after_success: npm run coveralls
after_install: npm run copy-metafiles && ./tests/dep-time-travel.sh && cd ${{ matrix.package }} && npm install
skip-ls-check: true
- run: cd ${{ matrix.package }} && npm run tests-only

packages:
name: 'packages: all tests'
Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,13 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
- [`no-extraneous-dependencies`]: Exclude flow `typeof` imports ([#1534], thanks [@devongovett])
- [`newline-after-import`]: respect decorator annotations ([#1985], thanks [@lilling])
- [`no-restricted-paths`]: enhance performance for zones with `except` paths ([#2022], thanks [@malykhinvi])
- [`no-unresolved`]: check import() ([#2026], thanks [@aladdin-add])

### Changed
- [Generic Import Callback] Make callback for all imports once in rules ([#1237], thanks [@ljqx])
- [Docs] [`no-named-as-default`]: add semicolon ([#1897], thanks [@bicstone])
- [Docs] `no-extraneous-dependencies`: correct peerDependencies option default to `true` ([#1993], thanks [@dwardu])
- [Docs] `order`: Document options required to match ordering example ([#1992], thanks [@silviogutierrez])

## [2.22.1] - 2020-09-27
### Fixed
Expand Down Expand Up @@ -764,6 +766,7 @@ for info on changes for earlier releases.

[`memo-parser`]: ./memo-parser/README.md

[#2026]: https://github.com/benmosher/eslint-plugin-import/pull/2026
[#2022]: https://github.com/benmosher/eslint-plugin-import/pull/2022
[#2021]: https://github.com/benmosher/eslint-plugin-import/pull/2021
[#1997]: https://github.com/benmosher/eslint-plugin-import/pull/1997
Expand Down Expand Up @@ -1355,3 +1358,5 @@ for info on changes for earlier releases.
[@s-h-a-d-o-w]: https://github.com/s-h-a-d-o-w
[@grit96]: https://github.com/grit96
[@lilling]: https://github.com/lilling
[@silviogutierrez]: https://github.com/silviogutierrez
[@aladdin-add]: https://github.com/aladdin-add
3 changes: 2 additions & 1 deletion docs/rules/order.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

Enforce a convention in the order of `require()` / `import` statements.
+(fixable) The `--fix` option on the [command line] automatically fixes problems reported by this rule.
The order is as shown in the following example:

With the [`groups`](#groups-array) option set to `["builtin", "external", "internal", "parent", "sibling", "index", "object"]` the order is as shown in the following example:

```js
// 1. node "builtin" modules
Expand Down
3 changes: 3 additions & 0 deletions resolvers/webpack/.eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,7 @@
"import/no-extraneous-dependencies": 1,
"no-console": 1,
},
"env": {
"es6": true,
},
}
10 changes: 8 additions & 2 deletions resolvers/webpack/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel

## Unreleased

### Added
- add support for webpack5 'externals function' ([#2023], thanks [@jet2jet])

### Changed
- Add warning about async Webpack configs ([#1962], thanks [@ogonkov])
- Add warning about async Webpack configs ([#1962], thanks [@ogonkov])
- Replace node-libs-browser with is-core-module ([#1967], thanks [@andersk])
- [meta] add "engines" field to document existing requirements

## 0.13.0 - 2020-09-27

Expand Down Expand Up @@ -141,6 +145,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
- `interpret` configs (such as `.babel.js`).
Thanks to [@gausie] for the initial PR ([#164], ages ago! 😅) and [@jquense] for tests ([#278]).

[#2023]: https://github.com/benmosher/eslint-plugin-import/pull/2023
[#1967]: https://github.com/benmosher/eslint-plugin-import/pull/1967
[#1962]: https://github.com/benmosher/eslint-plugin-import/pull/1962
[#1705]: https://github.com/benmosher/eslint-plugin-import/pull/1705
Expand Down Expand Up @@ -200,4 +205,5 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
[@migueloller]: https://github.com/migueloller
[@opichals]: https://github.com/opichals
[@andersk]: https://github.com/andersk
[@ogonkov]: https://github.com/ogonkov
[@ogonkov]: https://github.com/ogonkov
[@jet2jet]: https://github.com/jet2jet
47 changes: 40 additions & 7 deletions resolvers/webpack/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,14 @@ exports.resolve = function (source, file, settings) {

log('Using config: ', webpackConfig);

const resolveSync = getResolveSync(configPath, webpackConfig, cwd);

// externals
if (findExternal(source, webpackConfig.externals, path.dirname(file))) {
if (findExternal(source, webpackConfig.externals, path.dirname(file), resolveSync)) {
return { found: true, path: null };
}

// otherwise, resolve "normally"
const resolveSync = getResolveSync(configPath, webpackConfig, cwd);

try {
return { found: true, path: resolveSync(path.dirname(file), source) };
Expand Down Expand Up @@ -323,15 +324,15 @@ function makeRootPlugin(ModulesInRootPlugin, name, root) {
}
/* eslint-enable */

function findExternal(source, externals, context) {
function findExternal(source, externals, context, resolveSync) {
if (!externals) return false;

// string match
if (typeof externals === 'string') return (source === externals);

// array: recurse
if (Array.isArray(externals)) {
return externals.some(function (e) { return findExternal(source, e, context); });
return externals.some(function (e) { return findExternal(source, e, context, resolveSync); });
}

if (isRegex(externals)) {
Expand All @@ -340,13 +341,45 @@ function findExternal(source, externals, context) {

if (typeof externals === 'function') {
let functionExternalFound = false;
externals.call(null, context, source, function(err, value) {
const callback = function (err, value) {
if (err) {
functionExternalFound = false;
} else {
functionExternalFound = findExternal(source, value, context);
functionExternalFound = findExternal(source, value, context, resolveSync);
}
});
};
// - for prior webpack 5, 'externals function' uses 3 arguments
// - for webpack 5, the count of arguments is less than 3
if (externals.length === 3) {
externals.call(null, context, source, callback);
} else {
const ctx = {
context,
request: source,
contextInfo: {
issuer: '',
issuerLayer: null,
compiler: '',
},
getResolve: () => (resolveContext, requestToResolve, cb) => {
if (cb) {
try {
cb(null, resolveSync(resolveContext, requestToResolve));
} catch (e) {
cb(e);
}
} else {
log('getResolve without callback not supported');
return Promise.reject(new Error('Not supported'));
}
},
};
const result = externals.call(null, ctx, callback);
// todo handling Promise object (using synchronous-promise package?)
if (result && typeof result.then === 'function') {
log('Asynchronous functions for externals not supported');
}
}
return functionExternalFound;
}

Expand Down
3 changes: 3 additions & 0 deletions resolvers/webpack/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,8 @@
"mocha": "^3.5.3",
"nyc": "^11.9.0",
"webpack": "https://gist.github.com/ljharb/9cdb687f3806f8e6cb8a365d0b7840eb"
},
"engines": {
"node": "^16 || ^15 || ^14 || ^13 || ^12 || ^11 || ^10 || ^9 || ^8 || ^7 || ^6"
}
}
33 changes: 33 additions & 0 deletions resolvers/webpack/test/externals.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,17 @@
const chai = require('chai');
const expect = chai.expect;
const path = require('path');
const semver = require('semver');

const webpack = require('../index');

const file = path.join(__dirname, 'files', 'dummy.js');

describe('externals', function () {
const settingsWebpack5 = {
config: require(path.join(__dirname, './files/webpack.config.webpack5.js')),
};

it('works on just a string', function () {
const resolved = webpack.resolve('bootstrap', file);
expect(resolved).to.have.property('found', true);
Expand All @@ -32,4 +37,32 @@ describe('externals', function () {
expect(resolved).to.have.property('found', true);
expect(resolved).to.have.property('path', null);
});

it('works on a function (synchronous) for webpack 5', function () {
const resolved = webpack.resolve('underscore', file, settingsWebpack5);
expect(resolved).to.have.property('found', true);
expect(resolved).to.have.property('path', null);
});

it('works on a function (synchronous) which uses getResolve for webpack 5', function () {
const resolved = webpack.resolve('graphql', file, settingsWebpack5);
expect(resolved).to.have.property('found', true);
expect(resolved).to.have.property('path', null);
});

(semver.satisfies(process.version, '> 6') ? describe : describe.skip)('async function in webpack 5', function () {
const settingsWebpack5Async = () => ({
config: require(path.join(__dirname, './files/webpack.config.webpack5.async-externals.js')),
});

it('prevents using an asynchronous function for webpack 5', function () {
const resolved = webpack.resolve('underscore', file, settingsWebpack5Async());
expect(resolved).to.have.property('found', false);
});

it('prevents using a function which uses Promise returned by getResolve for webpack 5', function () {
const resolved = webpack.resolve('graphql', file, settingsWebpack5Async());
expect(resolved).to.have.property('found', false);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
module.exports = {
externals: [
{ 'jquery': 'jQuery' },
'bootstrap',
async function ({ request },) {
if (request === 'underscore') {
return 'underscore'
}
},
function ({ request, getResolve }, callback) {
if (request === 'graphql') {
const resolve = getResolve()
// dummy call (some-module should be resolved on __dirname)
resolve(__dirname, 'some-module').then(
function () { callback(null, 'graphql') },
function (e) { callback(e) }
)
}
},
],
}
27 changes: 27 additions & 0 deletions resolvers/webpack/test/files/webpack.config.webpack5.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
module.exports = {
externals: [
{ 'jquery': 'jQuery' },
'bootstrap',
function ({ request }, callback) {
if (request === 'underscore') {
return callback(null, 'underscore')
}
callback()
},
function ({ request, getResolve }, callback) {
if (request === 'graphql') {
const resolve = getResolve()
// dummy call (some-module should be resolved on __dirname)
resolve(__dirname, 'some-module', function (err, value) {
if (err) {
callback(err)
} else {
callback(null, 'graphql')
}
})
} else {
callback()
}
},
],
}
Loading