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

Odd behavior with twin.macro import #167

Open
therealgilles opened this issue Apr 12, 2024 · 9 comments
Open

Odd behavior with twin.macro import #167

therealgilles opened this issue Apr 12, 2024 · 9 comments
Assignees
Labels
bug Something isn't working needs reproduction Awaiting a minimal reproduction for further investigation

Comments

@therealgilles
Copy link

therealgilles commented Apr 12, 2024

Your Environment

  • Prettier version: 3.2.5
  • node version: 20.5.0
  • package manager: [email protected]
  • IDE: VSCode

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
Copy link
Collaborator

fbartho commented Apr 12, 2024

@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.

@fbartho fbartho added the question Further information is requested label Apr 12, 2024
@therealgilles
Copy link
Author

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
Copy link
Collaborator

fbartho commented Apr 12, 2024

@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:

- Does not re-order across side-effect imports

A pragma to permit it would be pretty complex code.

@therealgilles
Copy link
Author

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

@therealgilles
Copy link
Author

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
Copy link
Collaborator

fbartho commented Apr 12, 2024

@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
Copy link
Author

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
Copy link
Author

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

@IanVS
Copy link
Owner

IanVS commented Jun 25, 2024

@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.

@IanVS IanVS added bug Something isn't working needs reproduction Awaiting a minimal reproduction for further investigation and removed question Further information is requested labels Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs reproduction Awaiting a minimal reproduction for further investigation
Projects
None yet
Development

No branches or pull requests

3 participants