Auto-import fails when a identifier is the same as a name to be imported
Closed this issue · 4 comments
Environment
unimport version: v3.3.0
node.js version: v18.16.0
Reproduction
import { createUnimport } from 'unimport'
const code = `
{
const fn = () => {};
};
fn();
`
const { injectImports } = createUnimport({
imports: [{ name: 'fn', from: 'module' }]
})
const { imports } = await injectImports(code)
console.log(imports)
Describe the bug
Lines 247 to 256 in 28cf011
Before Remove those already defined
, the value of occurrenceMap
is { size:1, [{ "fn" => 30 }] }
, where fn
refers to the function which is called outside of the block scope.
While run into the for loop, strippedCode
will match regexp /\b(?:const|let|var)\s+?(\[.*?\]|\{.*?\}|.+?)\s*?[=;\n]/gs
, and matching result is [ "const fn =", "fn" ]
, where fn
refers to the function defined inside the block scope. So occurrenceMap.delete(identifier)
will be executed, occurrenceMap
will be cleared, and finally no import
statement will be prepended to the code.
Seems in this situation, regexp matching is not enough, and lexical analysis may be needed?
Additional context
No response
Logs
No response
We've also encountered this… I wonder if the only way to make unimport
work reliably would be to stop using regexps and use AST parsing and processing instead?
Related: #250 (comment)
@antfu what do you think? We keep seeing auto-import issues every now and then in our large code-base, and I can't always figure out why, or find the time to create a bug report. Most of the time, I do changes where I then rename a variable (as here), or change the way the auto-imported function is used. It's not exactly inspiring confidence…
Yeah, this is indeed sometimes impossible to fix with regex. I think that makes sense to introduce an optional mode to do full parsing.