Incorrect "There should be no empty line within import group"

[alecf]

I get this message a lot, with no rhyme or reason as to why...

For this code:

import { Observable } from 'rxjs/Observable';
import 'rxjs/add/observable/interval';
import 'rxjs/add/observable/of';
import 'rxjs/add/operator/filter';
import 'rxjs/add/operator/map';
import 'rxjs/add/operator/mergeMap';
import 'rxjs/add/operator/throttle';

I get this warning: There should be no empty line within import group for line 1.

For this code:

import { getActiveProjectId } from 'uf/app/selectors';
import {
  makeGetAnalysisModuleInfoForProject,
  makeGetScenarioInfoForProject,
} from 'uf/projects/selectors';
import { addAppMessage } from 'uf/appservices/actions';
import { loadProject, updateProjectStatus } from 'uf/projects/actions';

I get the same warning on the `addAppMessage' line.

For this code:

// Bootstrap babel and sourcemaps.
require('source-map-support/register');
const universalWebpack = require('universal-webpack');

const universalWebpackSettings = require('../webpack/universal-webpack-settings');
const webpackConfig = require('../webpack/webpack.config');

// This tells webpack to not re-apply babel to the server itself as a
// part of server startup.
const babelOptions = universalWebpack.babelRegisterOptions(
  universalWebpackSettings, webpackConfig);

I get the warning on the const webpackConfig = line.

I don't know if the error message is just on the wrong line, but I can't figure out how to correct my imports to make this pass.

[alecf]

this is with eslint-plugin-import 2.10.0

[ljharb]

What happens if you add a blank line between a side-effecting import/require and its neighboring normal import/require?

[DanielSWolf]

I'm experiencing a similar problem. This is my code:

import './polyfills';

import 'sqlite-evcustom-browser-memory-storage-ext-free';

import { version } from '@prd-crossmip/frame/package.json';
import dbUrl from '@prd-crossmip/frame/sandbox/mip.db';
import { registerBindings } from '@prd-crossmip/inject-registration'; // <1>
import '@prd-crossmip/ui-theme/src/basic.scss';

import './global.css';

import { getMipDatabaseBackend } from '@prd-crossmip/database'; // <2>

import { initTenants } from './tenants';

• At <1>, I'm getting the (incorrect) error "There should be no empty line within import group".
• At <2>, I'm getting the (correct) error "@prd-crossmip/database import should occur before import of @prd-crossmip/frame/package.json".

The auto-fixer doesn't do anything. My guess is that something about this order of imports puts it in an invalid state where it is unable to perform any fixes.

If I manually move import <2> to the correct position, everything works fine again. The incorrect error disappears and the auto-fixer works again.

[ljharb]

@DanielSWolf bindingless imports are assumed to be side-effecting, and thus can't ever be safely reordered (or anything moved across their boundary). That means you have to manually fix the ordering, if possible. This is by design and practically speaking, can't ever be changed.

In other words, if you moved the global.css import to the top or the bottom, the autofixer should work fine.

[DanielSWolf]

@ljharb That's a good point, thank you! It still doesn't explain the phantom error at <1>, though.

[ljharb]

@DanielSWolf what is your eslint config for this rule?

[DanielSWolf]

@ljharb Here's a stripped-down ESLint configuration that reproduces the phantom error:

module.exports = {
  parser: '@typescript-eslint/parser',
  plugins: ['import'],
  rules: {
    'import/order': [
      'error',
      {
        'alphabetize': { order: 'asc' },
        'groups': [
          'builtin',
          'external',
          'internal',
          'parent',
          ['index', 'sibling'],
          'object',
        ],
        'newlines-between': 'always',
        'pathGroups': [
          {
            pattern: '@prd-crossmip/**',
            group: 'external',
            position: 'after',
          },
          {
            pattern: '.*/**/*.scss',
            group: 'sibling',
            position: 'after',
          },
        ],
        'pathGroupsExcludedImportTypes': [],
      },
    ],
  },
};

[RemyMachado]

I have the same problem with the typescript resolver baseUrl. One line out of dozens doesn't get ordered correctly.

[ljharb]

ok! i've definitely turned the invalid one into a reproducible test case. @DanielSWolf can you show me the code matching https://github.com/import-js/eslint-plugin-import/issues/1062#issuecomment-805591110 that is both correct and also doesn't warn?

[Sergei8888]

Still getting this error on general usage of rule

[ljharb]

@Sergei8888 can you answer the question in https://github.com/import-js/eslint-plugin-import/issues/1062#issuecomment-903051219 ?

[loucadufault]

bindingless imports are assumed to be side-effecting, and thus can't ever be safely reordered (or anything moved across their boundary). That means you have to manually fix the ordering, if possible. This is by design and practically speaking, can't ever be changed.

IMO this should be reported in the error message itself, as to why the lint error is not fixable and what manual fix is required

As a side note, i've noticed eslint has no problem reordering other imports below the side-effecting imports, which could still be problematic.

[ljharb]

It shouldn't be problematic to reorder imports that have bindings.

As for the error message, I'm happy to review a PR that makes the message more helpful, bearing in mind that it shouldn't be overly verbose.

[loucadufault]

Maybe I am missing something in your response, but I don't see how imports with bindings would mitigate such an issue. Trivial repro:

// side-effecting.js
global.cannotExistYet = 'now it does'

// some-module.js
if (global.cannotExistYet) throw Error()

exports.someProp = 'foo'

// index.js

// this works
const { someBinding } = require('./imported')
require('./side-effecting')

// it would fail if eslint reorders the imports to
require('./side-effecting')
const { someBinding } = require('./imported')

Although i suppose this is an inherent danger with any import reordering, but maybe more likely to be problematic with side-effect only imports?

[ljharb]

@loucadufault yes - certainly every module might have side effects, whether it has a binding or not. If we assume that, then this rule simply can't exist at all.

However, the overwhelming convention is that the only modules that have side effects are the ones that export nothing, which is what this rule assumes.

[loucadufault]

With that said, is the example above not proof that reordering imports having a binding after a side-effecting import (by convention) is problematic? I am not sure there is a strong argument as to why it is safe to move a side-effecting import before a binding import, but not vice-versa. I am sure in practice the dangers of the latter are more common, but there are several practical examples that come to mind where the former would be disruptive.

My understanding would be that side-effecting imports should form boundary(ies) within the module such that reordering would not be able to automatically reorder an import to cross a boundary in either direction

[ljharb]

I'm not sure what you're asking.

If we do not accept the convention, the rule should not exist at all.

If we accept the convention, then the only things that would depend on side effects (by importing side-effecting module B into module A) would be module A - meaning, module A is safe to reorder in its consumers, but the module B import inside A is not.

[loucadufault]

I agree that it is unsafe to reorder "the module B import inside A" in your example above. Extending that, it must be unsafe to reorder the side-effecting import in either direction (whether it be hoisting it up or pushing it down).

However, the reordering has no problem with moving imports having a binding from above to below a side-effecting import, which is what I had described in my initial comment. This could be problematic if those reordered (non-side effecting) imports within the same module depend on those side effects (or more precisely depend on those side-effects not having yet happened).

[ljharb]

Yes, that's right - every side-effecting import is treated as a "boundary" across which no automatic sorting can occur.

Autofix moving a binding from above to below a side-effecting import would be a bug.

[loucadufault]

edit: Opened a bug report: #2876

[olawalejuwonm]

What's the resolution to this please?