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

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

Open
craigcosmo opened this issue Sep 4, 2024 · 2 comments

Comments

@craigcosmo
Copy link

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
Copy link

7nik commented Sep 4, 2024

See vitejs/vite#15890 for the info.
Maybe the svelte compiler should drop unused imports but not imports itself.

@paoloricciuti
Copy link
Member

paoloricciuti commented Sep 4, 2024

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?

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