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

Only preserve directives already at the start of the program #198

Merged
merged 2 commits into from
Feb 23, 2023
Merged

Only preserve directives already at the start of the program #198

merged 2 commits into from
Feb 23, 2023

Conversation

c-dante
Copy link
Contributor

@c-dante c-dante commented Dec 22, 2022

See: #182 (comment)

The previous logic of extracting all directives regardless of scope and moving them to the top of the program breaks various valid usages of directives.

This change updates the directive handling logic such that only the ones affected by import sorting are handled:

  • The directive must be at the program root (direct child of a Program node)
  • All previous sibling nodes must be directives

This ensures that only the block of directives at the top of a program are extracted + re-injected.

Previous behavior would transform this:

'use strict';
'use client';

import otherthing from "@core/otherthing";
import abc from "@core/abc";

// Comment
function add(a:number,b:number) {
    return a + b;
}
function addStrict(a:number,b:number) {
    'use strict';
    return a + b;
}

'preserve me';

const workletAdd = (a:number,b:number) => {
    'worklet';
    return a + b;
}

(function() {
    'use strict';
    // some iffe example
    return true;
})();

Into this:

'use strict';
'use client';
'use strict';
'preserve me';
'worklet';
'use strict';

import otherthing from "@core/otherthing";
import abc from "@core/abc";

// Comment
function add(a:number,b:number) {
    return a + b;
}
function addStrict(a:number,b:number) {
    return a + b;
}

const workletAdd = (a:number,b:number) => {
    return a + b;
}

(function() {
    // some iffe example
    return true;
})();

@broofa
Copy link
Contributor

broofa commented Dec 23, 2022

Good catch! Looks good to me.

@kaloyanBozhkov
Copy link

let's merge this!

@midrizi
Copy link

midrizi commented Jan 29, 2023

I was getting this issue as well, I thought it had to do with prettier, after removing trivago/prettier-plugin-sort-imports I didn't get the same behavior.

@midrizi
Copy link

midrizi commented Jan 29, 2023

Here's a quick patch, until it gets merged:

Use patch-package

diff --git a/node_modules/@trivago/prettier-plugin-sort-imports/lib/src/utils/extract-ast-nodes.js b/node_modules/@trivago/prettier-plugin-sort-imports/lib/src/utils/extract-ast-nodes.js
index 819a91c..54c1bb7 100644
--- a/node_modules/@trivago/prettier-plugin-sort-imports/lib/src/utils/extract-ast-nodes.js
+++ b/node_modules/@trivago/prettier-plugin-sort-imports/lib/src/utils/extract-ast-nodes.js
@@ -10,11 +10,17 @@ function extractASTNodes(ast) {
     var importNodes = [];
     var directives = [];
     traverse_1.default(ast, {
-        Directive: function (_a) {
-            var node = _a.node;
-            directives.push(node);
-            // Trailing comments probably shouldn't be attached to the directive
-            node.trailingComments = null;
+        Directive: function (path) {
+            // Only capture directives if they are at the top scope of the source
+            // and their previous siblings are all directives
+            if (path.parent.type === 'Program' &&
+                path.getAllPrevSiblings().every(function (s) {
+                    return s.type === 'Directive';
+                })) {
+                directives.push(path.node);
+                // Trailing comments probably shouldn't be attached to the directive
+                path.node.trailingComments = null;
+            }
         },
         ImportDeclaration: function (path) {
             var tsModuleParent = path.findParent(function (p) {

@DesignMonkey
Copy link

Good job :)

@byara byara merged commit 82fc5e3 into trivago:master Feb 23, 2023
@ayusharma
Copy link
Collaborator

Released in v4.1.0. I appreciate your patience. ❤️

@c-dante c-dante deleted the me/patch-directive branch February 25, 2023 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants