IanVS / prettier-plugin-sort-imports

An opinionated but flexible prettier plugin to sort import statements
Apache License 2.0
880 stars 21 forks source link

Odd behavior with `twin.macro` import #167

Open therealgilles opened 2 months ago

therealgilles commented 2 months ago

Your Environment

Describe the bug twin.macro seems to bother the sort algorithm. This is what it does:

import { useState } from 'react'
import { ApolloError, useMutation } from '@apollo/client'

import 'twin.macro'

import { v4 as uuidv4 } from 'uuid'

To Reproduce See above and config below.

Expected behavior I expect this:

import { useState } from 'react'
import { ApolloError, useMutation } from '@apollo/client'
import 'twin.macro'
import { v4 as uuidv4 } from 'uuid'

Configuration File (cat .prettierrc, prettier.config.js, .prettier.js) Here is the importOrder:

  importOrder: [
    '',
    '^react$',
    '<BUILT_IN_MODULES>',
    '<THIRD_PARTY_MODULES>',
    '^twin[.]macro$',
    '',
    '^@/',
    '^[.]',
  ],
fbartho commented 2 months ago

@therealgilles -- import 'twin.macro' is a "Side-effect Import" which is dangerous for us to reorganize, regardless of your regex. As a result, your importOrder isn't doing what you would like it to do.

This plugin will not reorganize "side-effect" imports.

therealgilles commented 2 months ago

I'm not sure I understand. The plugin is re-organizing its placement by adding blank spacing around it. Why not provide a pragma for side-effect imports, or better allow exact matches in the import list?

I would expect this to work for instance:

  importOrder: [
    '',
    '^react$',
    '^twin.macro$',
    '<BUILT_IN_MODULES>',
    '<THIRD_PARTY_MODULES>',
    '',
    '^@/',
    '^[.]',
  ],
fbartho commented 2 months ago

@therealgilles That's not a feature of this plugin. This plugin takes great care to be the least dangerous it can be, and reordering across side-effect imports could break many many projects -- Other import sorting plugins do offer that feature, this plugin chose not to. Making sure not to do so took a lot of code to get right, so that's a huge attractive factor for this plugin.

This is why we have the first bullet-point in the README: https://github.com/IanVS/prettier-plugin-sort-imports/blob/135b6e55d710df32c3a23c7fe266a04e1eb0c2c1/README.md?plain=1#L7

A pragma to permit it would be pretty complex code.

therealgilles commented 2 months ago

Gotcha! Thanks for taking the time to explain. It is not a big deal anyway.

therealgilles commented 2 months ago

I am still surprised the plugin is adding blank lines around the side-effect import. Why is that? Also for some reason, if I use // prettier-ignore, a semi-colon is added at the end of the import line, which seems unexpected.

fbartho commented 2 months ago

@therealgilles when you have a side-effect import, we treat the block above the side-effect and the block below the side-effect as independent groups (that are each sorted according to importOrder).

Your importOrder config includes 2 blank lines (above the first group, and between the 3rd-party imports and the local imports). I could be wrong because it's been a year since my last dive into the code, but I believe that's why it's inserting the blank lines you're referencing.


The semi-colon is indeed unexpected given what I assume is your prettier config. For myself I always use semi-colons in my code -- it's likely we didn't test that case heavily, but I would have expected prettier to respect your settings.

I can tell you that this plugin doesn't directly manipulate semi-colons, we just manipulate Abstract-syntax-tree nodes, and allow babel/prettier to assemble the final output according to your configs e.g. indentation, line lengths, and semi-colons are all handled outside of this plugin.

therealgilles commented 2 months ago

Okay got it, I appreciate the detailed explanation, thank you.

I am thinking the semi-colon may come from another setting somewhere, or another formatter.

therealgilles commented 2 months ago

I ran a couple experiments and the addition of the semi-colon only happens when this plugin is enabled.

IanVS commented 1 week ago

@therealgilles if you're still intersted, would you be willing to create a quick reproduction repo that we can look into? I'm not familiar with twin.macro personally.