import-js icon indicating copy to clipboard operation
import-js copied to clipboard

Allow some things to come before imports block

Open lencioni opened this issue 9 years ago • 5 comments

If you are using Jest, for instance, you might need some lines to appear before your imports block, such as calls to jest.unmock. Some people may even have these lines embedded within the imports block.

We might be able to add some configuration that would allow some things like this to appear before or within the imports block, but I'm not entirely sure what that would look like exactly to provide the flexibility that might be needed for this type of thing.

lencioni avatar Mar 27 '16 16:03 lencioni

I think the right way to fix this is to change the logic of where we start the import block.

  • If there are no previous imports, we start at the top (unless there's comments, use strict etc)
  • If there is one or more previous imports, start there.

Then users can manually "fix" the ordering once and have it kept after that.

trotzig avatar Mar 29 '16 06:03 trotzig

This sounds like a pretty good strategy. I think we also need to look only at import statements that are at the top level, since require can be used within functions. This will be easier once we switch to an AST-based approach.

The drawback is that if there are non-import calls within the import block, things get messy. Here's an example:

jest.unmock('something');
import foo from './foo';

jest.unmock('something-else');
import bar from './bar';

I'm sure there are other examples that can be found. This is also related to #215. This might just be a limitation that we are okay with, but it would be nice to find an elegant solution. Perhaps we should detect this scenario and disable sorting and put new imports after the last import?

lencioni avatar Mar 29 '16 23:03 lencioni

Yep, we should only consider top level imports.

Interesting example. What if we keep scanning until we find the last, top-level defined, import and then add our block of imports together with that import?

trotzig avatar Mar 30 '16 09:03 trotzig

Shebang for node scripts is another case I've run into recently.

#!/usr/bin/env node

lencioni avatar Apr 13 '16 15:04 lencioni

Note that there is some relationship between this issue and #215 Comments in import blocks.

I'm in agreement that we must limit ourselves to top-level imports. Note that Meteor supports "import" in conditionals and nested contexts. Our tech lead is a TC39 member and is bringing the subject of allowing nested imports back up with the committee this month.

But, I have also considered this top-level import after other code question in the past.

I believe we will encounter the case of projects that want to keep imports near the first code that depends on them. It is a relatively common pattern in other languages. This would cause imports to spread throughout the file. It argues for putting our imports near the first ones we find, not the last ones.

The gotcha I wonder about is in the case of detecting that another named export needs to be added to the list being imported from a package that already has an import further down in the file. Do we move that whole import to the top? Do either of you know whether the ES6 specification tolerates multiple import statements for the same package? Or perhaps it guarantees that no matter where you put an import in the file, its results are present throughout?

rhettlivingston avatar Jul 23 '16 22:07 rhettlivingston