sveltejs / svelte

web development for the rest of us
https://svelte.dev
MIT License
79.8k stars 4.24k forks source link

"default" is imported from external module "svelte-autosize" but never used #13122

Open craigcosmo opened 2 months ago

craigcosmo commented 2 months ago

Describe the bug

"default" is imported from external module "svelte-autosize" but never used

CleanShot 2024-09-04 at 11 46 10

Reproduction

<script lang="ts">
    import autosize from 'svelte-autosize'
</script>

<textarea use:autosize></textarea>

Logs

No response

System Info

Version: 1.92.1 (Universal)
Commit: eaa41d57266683296de7d118f574d0c2652e1fc4
Date: 2024-08-07T20:16:39.455Z
Electron: 30.1.2
ElectronBuildId: 9870757
Chromium: 124.0.6367.243
Node.js: 20.14.0
V8: 12.4.254.20-electron.0
OS: Darwin arm64 23.4.0

Severity

annoyance

7nik commented 2 months ago

See https://github.com/vitejs/vite/issues/15890 for the info. Maybe the svelte compiler should drop unused imports but not imports itself.

paoloricciuti commented 2 months ago

I started exploring this a bit but i really don't like the solution (which also is not fully coded yet). My solution was basically to do what rollup will do anyway and remove all the imports that doesn't have a reference in the outputted code.

Basically instead of returning the ast immediately we could do this

const all_imported_specifiers = new Set(
    body
        .filter((el) => el.type === 'ImportDeclaration')
        .flatMap((import_thing) => {
            return import_thing.specifiers.map((specifier) => specifier.local.name);
        })
);

/**
 * @type {{type: 'Program', sourceType: 'module', body: typeof body}}
 */
const returned_ast = {
    type: 'Program',
    sourceType: 'module',
    body
};

walk(
    returned_ast,
    {},
    {
        Identifier(node, context) {
            if (!context.path.some((parent) => parent.type === 'ImportDeclaration')) {
                all_imported_specifiers.delete(node.name);
            }
        }
    }
);

returned_ast.body = returned_ast.body.map((statement) => {
    if (statement.type !== 'ImportDeclaration') return statement;
    const updated_specifiers = statement.specifiers
        .map((specifier) => {
            if (all_imported_specifiers.has(specifier.local.name)) {
                return null;
            }
            return specifier;
        })
        .filter(Boolean);
    return {
        ...statement,
        specifiers: updated_specifiers
    };
});

return returned_ast;

however this is very likely to have a lot of edge cases because of how svelte produces the output code: for the ast for an internal function calls the identifiers generally look like this

{
    type: "Identifier",
    name: "$.each",
}

instead of having a member access for the $ identifier (which to be fair it's the sanest thing to do to make the visitors code readable). So basically the $ identifier is actually never there. We could make a special case for the $ identifier since we know it will be used and can only be svelte but i fear that there might be some edge case right behind the corner.

Considering this is basically doing work that rollup will already do (is just to mute the warning) and that it's not actually blocking (because it's just a warning)...do you think is worth it to explore?