fix(vite): skip generating keys for locally scoped functions (#20955)

packages/schema/src/config/build.ts

@@ -142,7 +142,7 @@ export default defineUntypedSchema({
      *
      * The key will be unique based on the location of the function being invoked within the file.
      *
-     * @type {Array<{ name: string, argumentLength: number }>}
+     * @type {Array<{ name: string, source?: string | RegExp, argumentLength: number }>}
      */
     keyedComposables: {
       $resolve: (val) => [

packages/vite/src/plugins/composable-keys.ts

@@ -5,15 +5,16 @@ import type { Node } from 'estree-walker'
 import { walk } from 'estree-walker'
 import MagicString from 'magic-string'
 import { hash } from 'ohash'
-import type { CallExpression } from 'estree'
+import type { CallExpression, Pattern } from 'estree'
 import { parseQuery, parseURL } from 'ufo'
 import escapeRE from 'escape-string-regexp'
 import { findStaticImports, parseStaticImport } from 'mlly'
+import { matchWithStringOrRegex } from '../utils'
 
 export interface ComposableKeysOptions {
   sourcemap: boolean
   rootDir: string
-  composables: Array<{ name: string, argumentLength: number }>
+  composables: Array<{ name: string, source?: string | RegExp, argumentLength: number }>
 }
 
 const stringTypes = ['Literal', 'TemplateLiteral']
@@ -42,21 +43,60 @@ export const composableKeysPlugin = createUnplugin((options: ComposableKeysOptio
       let imports: Set<string> | undefined
       let count = 0
       const relativeID = isAbsolute(id) ? relative(options.rootDir, id) : id
-      walk(this.parse(script, {
+      const { pathname: relativePathname } = parseURL(relativeID)
+
+      const ast = this.parse(script, {
         sourceType: 'module',
         ecmaVersion: 'latest'
-      }) as Node, {
+      }) as Node
+
+      // To handle variables hoisting we need a pre-pass to collect variable and function declarations with scope info.
+      let scopeTracker = new ScopeTracker()
+      const varCollector = new ScopedVarsCollector()
+      walk(ast, {
         enter (_node) {
+          if (_node.type === 'BlockStatement') {
+            scopeTracker.enterScope()
+            varCollector.refresh(scopeTracker.curScopeKey)
+          } else if (_node.type === 'FunctionDeclaration' && _node.id) {
+            varCollector.addVar(_node.id.name)
+          } else if (_node.type === 'VariableDeclarator') {
+            varCollector.collect(_node.id)
+          }
+        },
+        leave (_node) {
+          if (_node.type === 'BlockStatement') {
+            scopeTracker.leaveScope()
+            varCollector.refresh(scopeTracker.curScopeKey)
+          }
+        }
+      })
+
+      scopeTracker = new ScopeTracker()
+      walk(ast, {
+        enter (_node) {
+          if (_node.type === 'BlockStatement') {
+            scopeTracker.enterScope()
+          }
           if (_node.type !== 'CallExpression' || (_node as CallExpression).callee.type !== 'Identifier') { return }
           const node: CallExpression = _node as CallExpression
           const name = 'name' in node.callee && node.callee.name
           if (!name || !keyedFunctions.has(name) || node.arguments.length >= maxLength) { return }
 
-          imports = imports || detectImportNames(script)
+          imports = imports || detectImportNames(script, composableMeta)
           if (imports.has(name)) { return }
 
           const meta = composableMeta[name]
 
+          if (varCollector.hasVar(scopeTracker.curScopeKey, name)) {
+            let skip = true
+            if (meta.source) {
+              skip = !matchWithStringOrRegex(relativePathname, meta.source)
+            }
+
+            if (skip) { return }
+          }
+
           if (node.arguments.length >= meta.argumentLength) { return }
 
           switch (name) {
@@ -82,6 +122,11 @@ export const composableKeysPlugin = createUnplugin((options: ComposableKeysOptio
             codeIndex + (node as any).end - 1,
             (node.arguments.length && !endsWithComma ? ', ' : '') + "'$" + hash(`${relativeID}-${++count}`) + "'"
           )
+        },
+        leave (_node) {
+          if (_node.type === 'BlockStatement') {
+            scopeTracker.leaveScope()
+          }
         }
       })
       if (s.hasChanged()) {
@@ -96,22 +141,112 @@ export const composableKeysPlugin = createUnplugin((options: ComposableKeysOptio
   }
 })
 
+class ScopeTracker {
+  scopeIndexStack: number[]
+  curScopeKey: string
+
+  constructor () {
+    // top level
+    this.scopeIndexStack = [0]
+    this.curScopeKey = '0'
+  }
+
+  getKey () {
+    return this.scopeIndexStack.slice(0, -1).join('-')
+  }
+
+  enterScope () {
+    this.scopeIndexStack.push(0)
+    this.curScopeKey = this.getKey()
+  }
+
+  leaveScope () {
+    this.scopeIndexStack.pop()
+    this.curScopeKey = this.getKey()
+    this.scopeIndexStack[this.scopeIndexStack.length - 1]++
+  }
+}
+
+class ScopedVarsCollector {
+  curScopeKey: string
+  all: Map<string, Set<string>>
+
+  constructor () {
+    this.all = new Map()
+    // top level
+    this.curScopeKey = '0'
+  }
+
+  refresh (scopeKey: string) {
+    this.curScopeKey = scopeKey
+  }
+
+  addVar (name: string) {
+    let vars = this.all.get(this.curScopeKey)
+    if (!vars) {
+      vars = new Set()
+      this.all.set(this.curScopeKey, vars)
+    }
+    vars.add(name)
+  }
+
+  hasVar (scopeKey: string, name: string) {
+    const indices = scopeKey.split('-').map(Number)
+    for (let i = indices.length; i > 0; i--) {
+      if (this.all.get(indices.slice(0, i).join('-'))?.has(name)) {
+        return true
+      }
+    }
+    return false
+  }
+
+  collect (n: Pattern) {
+    const t = n.type
+    if (t === 'Identifier') {
+      this.addVar(n.name)
+    } else if (t === 'RestElement') {
+      this.collect(n.argument)
+    } else if (t === 'AssignmentPattern') {
+      this.collect(n.left)
+    } else if (t === 'ArrayPattern') {
+      n.elements.forEach(e => e && this.collect(e))
+    } else if (t === 'ObjectPattern') {
+      n.properties.forEach((p) => {
+        if (p.type === 'RestElement') {
+          this.collect(p)
+        } else {
+          this.collect(p.value)
+        }
+      })
+    }
+  }
+}
+
 const NUXT_IMPORT_RE = /nuxt|#app|#imports/
 
-function detectImportNames (code: string) {
+function detectImportNames (code: string, composableMeta: Record<string, { source?: string | RegExp }>) {
   const imports = findStaticImports(code)
   const names = new Set<string>()
   for (const i of imports) {
     if (NUXT_IMPORT_RE.test(i.specifier)) { continue }
-    const { namedImports, defaultImport, namespacedImport } = parseStaticImport(i)
+
-    for (const name in namedImports || {}) {
+    function addName (name: string) {
+      const source = composableMeta[name]?.source
+      if (source && matchWithStringOrRegex(i.specifier, source)) {
+        return
+      }
       names.add(namedImports![name])
     }
+
+    const { namedImports, defaultImport, namespacedImport } = parseStaticImport(i)
+    for (const name in namedImports || {}) {
+      addName(namedImports![name])
+    }
     if (defaultImport) {
-      names.add(defaultImport)
+      addName(defaultImport)
     }
     if (namespacedImport) {
-      names.add(namespacedImport)
+      addName(namespacedImport)
     }
   }
   return names

packages/vite/src/utils/index.ts

@@ -43,3 +43,13 @@ export async function isDirectory (path: string) {
     return false
   }
 }
+
+export function matchWithStringOrRegex (value: string, matcher: string | RegExp) {
+  if (typeof matcher === 'string') {
+    return value === matcher
+  } else if (matcher instanceof RegExp) {
+    return matcher.test(value)
+  }
+
+  return false
+}

test/basic.test.ts

@@ -1086,6 +1086,12 @@ describe('automatically keyed composables', () => {
   it('should match server-generated keys', async () => {
     await expectNoClientErrors('/keyed-composables')
   })
+  it('should not automatically generate keys', async () => {
+    await expectNoClientErrors('/keyed-composables/local')
+    const html = await $fetch('/keyed-composables/local')
+    expect(html).toContain('true')
+    expect(html).not.toContain('false')
+  })
 })
 
 describe.skipIf(isDev() || isWebpack)('inlining component styles', () => {

test/fixtures/basic/nuxt.config.ts

@@ -62,7 +62,8 @@ export default defineNuxtConfig({
   optimization: {
     keyedComposables: [
       {
-        name: 'useKeyedComposable',
+        name: 'useCustomKeyedComposable',
+        source: 'pages/keyed-composables/index.vue',
         argumentLength: 1
       }
     ]

test/fixtures/basic/pages/keyed-composables/index.vue

@@ -33,10 +33,10 @@ const useLocalLazyFetch = () => useLazyFetch(() => '/api/counter')
 const { data: useLazyFetchTest1 } = await useLocalLazyFetch()
 const { data: useLazyFetchTest2 } = await useLocalLazyFetch()
 
-const useKeyedComposable = (arg?: string) => arg
+const useCustomKeyedComposable = (arg?: string) => arg
-const useLocalKeyedComposable = () => useKeyedComposable()
+const useLocalCustomKeyedComposable = () => useCustomKeyedComposable()
-const useMyAsyncDataTest1 = useLocalKeyedComposable()
+const useMyAsyncDataTest1 = useLocalCustomKeyedComposable()
-const useMyAsyncDataTest2 = useLocalKeyedComposable()
+const useMyAsyncDataTest2 = useLocalCustomKeyedComposable()
 </script>
 
 <template>

test/fixtures/basic/pages/keyed-composables/local.vue

@@ -0,0 +1,59 @@
+<script setup lang="ts">
+function localScopedComposables () {
+  const _assert = (key?: string) => key ?? 'was not keyed'
+
+  function basic () {
+    function useState (key?: string) {
+      return _assert(key)
+    }
+    const useAsyncData = _assert
+
+    return [useState(), useAsyncData()]
+  }
+
+  function hoisting () {
+    return [useState()]
+
+    function useState (key?: string) {
+      return _assert(key)
+    }
+  }
+
+  function complex () {
+    const [useState] = [_assert]
+    const { a: useAsyncData } = {
+      a: _assert
+    }
+    const [_, { b: useLazyAsyncData }] = [null, {
+      b: _assert
+    }]
+
+    return [useState(), useAsyncData(), useLazyAsyncData()]
+  }
+
+  function deeperScope () {
+    const useState = _assert
+
+    return [(function () {
+      return useState()
+    })()]
+  }
+
+  return [...basic(), ...hoisting(), ...complex(), ...deeperScope()]
+}
+
+const skippedLocalScopedComposables = localScopedComposables().every(res => res === 'was not keyed')
+</script>
+
+<template>
+  <div>
+    {{ skippedLocalScopedComposables }}
+  </div>
+</template>
+
+<style scoped>
+body {
+  background-color: #000;
+  color: #fff;
+}
+</style>