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.

Sign up for GitHub

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

Jump to bottom

'use strict' pushed below imports #5

Closed
1 task
BleedingDev opened this issue Apr 11, 2022 · 1 comment · Fixed by #6
Closed
1 task

'use strict' pushed below imports #5

BleedingDev opened this issue Apr 11, 2022 · 1 comment · Fixed by #6
Assignees
@blutorange @IanVS

Comments

@BleedingDev
Copy link

Your Environment

  • Prettier version: 2.6.2
  • node version: 16.14.0
  • package manager: 8.3.1
  • IDE: CLI

Describe the bug

'use strict' is pushed below imports.

To Reproduce

  1. Try to format this code:
'use strict'

import type { Period } from './Period'
  1. Bug appears.
import type { Period } from './Period'

;('use strict')

Expected behavior

Keep 'use strict' above.

Screenshots, code sample, etc

Configuration File (cat .prettierrc, prettier.config.js, .prettier.js)

{
  "semi": false,
  "trailingComma": "none",
  "singleQuote": true,
  "printWidth": 640,
  "importOrder": ["@riot/metaplayer", "^[./]"],
  "importOrderSeparation": true
}

Error log

Contribute to @trivago/prettier-plugin-sort-imports

  • I'm willing to fix this bug 🥇
@IanVS
Copy link
Owner

IanVS commented Apr 11, 2022

Hi, thanks for the report. Can I ask, why are you including use strict? ES Modules are always in strict mode, so AFAIK adding it explicitly has no effect.

@blutorange blutorange self-assigned this Apr 18, 2022
blutorange added a commit that referenced this issue Apr 18, 2022
@blutorange
Preserve directive prologues, fixes #5
b2fd863 
The most important goal is correctness. Directives were destroyed by placing
imports above it. Directive prologues are defined by the spec as

> A Directive Prologue is the longest sequence of ExpressionStatements occurring as
> the initial StatementListItems or ModuleItems of a FunctionBody, a ScriptBody, or
> a ModuleBody and where each ExpressionStatement in the sequence consists entirely
> of a StringLiteral token followed by a semicolon. The semicolon may appear explicitly
> or may be inserted by automatic semicolon insertion (12.9). A Directive Prologue may
> be an empty sequence.
@blutorange blutorange mentioned this issue Apr 18, 2022
Merged
blutorange added a commit that referenced this issue Apr 18, 2022
@blutorange
Remove comments from directives we add back at the top, #5
7cb3560 
The comments are added back as part of the new directives placed at the top, so
we should remove them from the original code we add after the directives and the
imports.
@IanVS IanVS closed this as completed in #6 Apr 27, 2022
IanVS pushed a commit that referenced this issue Apr 27, 2022
@blutorange
Preserve directive prologues fixes #5 #7 (#6)
b5c23c7 
* Preserve directive prologues, fixes #5

The most important goal is correctness. Directives were destroyed by placing
imports above it. Directive prologues are defined by the spec as

> A Directive Prologue is the longest sequence of ExpressionStatements occurring as
> the initial StatementListItems or ModuleItems of a FunctionBody, a ScriptBody, or
> a ModuleBody and where each ExpressionStatement in the sequence consists entirely
> of a StringLiteral token followed by a semicolon. The semicolon may appear explicitly
> or may be inserted by automatic semicolon insertion (12.9). A Directive Prologue may
> be an empty sequence.

* Remove nodes from code via proper algorithm, fixes #7

Previously, a regex string replacement was used, and regex is an inadequate
tool for transforming entire JavaScript code files. Instead, this commit
changes that to a proper algorithm that removes the code at the source position
ranges of the nodes to be removed. This will not work properly if the Babel
parser ever did not return the correct source positions, but that is not a
regression since the former algortihm already relied on the positions
being correct.

* Remove comments from directives we add back at the top, #5

The comments are added back as part of the new directives placed at the top, so
we should remove them from the original code we add after the directives and the
imports.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@blutorange blutorange

@IanVS IanVS

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Preserve directive prologues fixes #5 #7 IanVS/prettier-plugin-sort-imports
3 participants
@blutorange @IanVS @BleedingDev