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

Vue SFC and imports are not gracefully handled per-block when using script and script setup #177

Open
cyyynthia opened this issue Nov 5, 2024 · 2 comments

Comments

@cyyynthia
Copy link

Given the following code:

<script lang="ts">
console.log('a') 
import 'vue'
console.log('b')
</script>

<script setup lang="ts">
console.log('c') 
import 'vue'
console.log('d')
</script>

The import rules fail to handle both of the script blocks as distinct scripts. Specifically in this case, import-x/first will not produce the expected results and attempt to group all imports under a same script block.

I did a hacky patch via pnpm to overcome this issue in the project I'm using, I'm not sure it's the "proper" way but it did the trick just fine in my case. I've provided the patch below.

I'm afraid I don't have enough time to clean/polish/test the patch and file it as a PR myself, sorry. I hope it'll be useful nonetheless! 😅

eslint-plugin-import-x.patch
diff --git a/lib/rules/first.js b/lib/rules/first.js
index 330f6612d409e7f2cc3f0fe86d3cb4b9ae7f2c7a..0e08cc0d13d026e17e528fb43e1942bfe2c97ed1 100644
--- a/lib/rules/first.js
+++ b/lib/rules/first.js
@@ -35,12 +35,42 @@ module.exports = (0, utils_1.createRule)({
     },
     defaultOptions: [],
     create(context) {
+        // [Cynthia] Begin patch: Vue support
+        const __cyn_isVue = context.filename.endsWith('.vue')
+        // [Cynthia] End patch: Vue support
+
         return {
             Program(n) {
-                const body = n.body;
-                if (!body?.length) {
-                    return;
+                // [Cynthia] Begin patch: Vue support
+                if (!n.body?.length) return
+
+                const __cyn_bodies = []
+                if (__cyn_isVue) {
+                    const __cyn_docFrag = context.sourceCode.parserServices.getDocumentFragment()
+                    const __cyn_scriptRanges = __cyn_docFrag.children
+                        .filter((c) => c.type === 'VElement' && c.name === 'script')
+                        .map((c) => c.range)
+
+                    for (const __cyn_range of __cyn_scriptRanges) {
+                        const __cyn_body = []
+                        for (const __cyn_node of n.body) {
+                            if (__cyn_node.range[0] > __cyn_range[0] && __cyn_node.range[1] < __cyn_range[1]) {
+                              __cyn_body.push(__cyn_node)
+                            }
+                        }
+
+                        if (__cyn_body.length) {
+                            __cyn_bodies.push(__cyn_body)
+                        }
+                    }
+                } else {
+                    __cyn_bodies.push(n.body)
                 }
+
+                for (const body of __cyn_bodies) {
+                ;(() => { // IIFE to void undesired `return` and make them behave as `continue`
+                // [Cynthia] End patch: Vue support
+
                 const absoluteFirst = context.options[0] === 'absolute-first';
                 const { sourceCode } = context;
                 const originSourceCode = sourceCode.getText();
@@ -147,6 +177,11 @@ module.exports = (0, utils_1.createRule)({
                         fix,
                     });
                 }
+
+                // [Cynthia] Begin patch: Vue support
+                })();
+                }
+                // [Cynthia] End patch: Vue support
             },
         };
     },
@SukkaW
Copy link
Collaborator

SukkaW commented Nov 11, 2024

We are more than happy to accept PRs for this!

@Kenneth-Sills
Copy link

@cyyynthia Thanks for reporting this!

I wouldn't actually consider this a concern of eslint-plugin-import-x itself, more of the vue-eslint-parser project not hoisting import statements in <script setup> correctly. I've created a ticket in that project here, which would inherently fix this issue.

That said, I think that the downstream source would probably be a more appropriate place to fix this. All imports and globals in <script> execute before those of <script setup> and are fully available, so imports can be hoisted from <setup script> to <script>`.

<script>
import { normalize } from 'node:path';
import { readFileSync } from 'node:fs';

// Use normalize
</script>

<script setup>
// Use readFileSync
</script>

This keeps all your import declarations at the top of the file, which is generally the best practice. That would also avoid other issues, like import-x/no-duplicates being generated by:

<script>
import { normalize } from 'node:path';
</script>

<script setup>
import { basename } from 'node:path';
</script>

Or a fatal duplicate identifier error happening from:

<script>
import { normalize } from 'node:path';
</script>

<script setup>
import { normalize } from 'node:path';
</script>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants