Added option to keep_imports

[aleph1]

As per #1420, I have added a mangle option of keep_imports. I know the readme will need updating, but am unsure if anything needs to be changed with the CLI, or whether I should be adding any additional tests.

[fabiosantoscode]

Would appreciate a test there. You can use this linked test as a template

https://github.com/terser/terser/blob/master/test/compress/mangleprops-computed.js#L1-L22

Use options = { defaults: false } to disable compression, and mangle = { keep_imports: true }.

You can run these locally with npm run test:compress

[fabiosantoscode]

I don't think you'll need to add something to cli.js, as it just parses options and passes them on. But TypeScript users will appreciate a change here

[fabiosantoscode]

Also I'd like to know what you're working on! For some small number of import usages I can see gzip treating straight-up imports more efficiently than a single letter plus as x. But I can imagine someone building a more clever gzip specifically for compressing JS files. I'm wondering if that's what you're working on.

[aleph1]

I’ll aim to make those other changes by early next week. I’m working on a gui library that has a large number of imports and the byte shavings off the production build are not insignificant, though a more clever gzip for js files sounds like it could be a good idea.

…

On Aug 4, 2023, at 9:38 AM, Fábio Santos ***@***.***> wrote:  Also I'd like to know what you're working on! For some small number of import usages I can see gzip treating straight-up imports more efficiently than a single letter plus as x. But I can imagine someone building a more clever gzip specifically for compressing JS files. I'm wondering if that's what you're working on. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

[aleph1]

But TypeScript users will appreciate a change [here](https://github.com/terser/terser/blob/master/tools/terser.d.ts#L79-L89)

Looking at the TypeScript definitions, I noticed that keep_classnames and keep_fnames can be booleans or RegExps. Would it make sense for me to implement keep_imports so that it uses the same types and logic versus the implementation in the pull request, which is boolean only?

[fabiosantoscode]

As per your comment, maybe what's needed to solve the problem is a heuristic such as keeping imports that are used less than 2 or 3 times. this.references.length should contain this information.

[fabiosantoscode]

Would it make sense for me to implement keep_imports so that it uses the same types and logic versus the implementation in the pull request, which is boolean only?

I can take it or leave it :) if someone needs a regex keep_imports, I can ask for another PR.

[aleph1]

Would appreciate a test there. You can use this linked test as a template

https://github.com/terser/terser/blob/master/test/compress/mangleprops-computed.js#L1-L22

Use options = { defaults: false } to disable compression, and mangle = { keep_imports: true }.

You can run these locally with npm run test:compress

When adding the tests I am getting the same result for the output in both of these cases, but the actual output when running terser with the same settings is different and the imported bindings are aliased as expected.

This test passes:

keep_imports_true_named: {
    options = {
        defaults: false
    }
    mangle = {
        keep_imports: true,
    }
    input: {
        import {test} from 'test'
    }
    expect: {
        import {test} from 'test'
    }
}

But the following test fails with the output import{test}from"test";:

keep_imports_false_named: {
    options = {
        defaults: false
    }
    mangle = {
        keep_imports: false,
    }
    input: {
        import {test} from 'test'
    }
    expect: {
        import {test as a} from 'test'
    }
}

[fabiosantoscode]

I think toplevel: true should be added to the mangle options. My bad!

[fabiosantoscode]

Looks good to merge! I can handle the docs.

I'm leaving it up to you whether you'd like to experiment with checking how often an import is used, before deciding whether to mangle, or if you think this is good to merge.

[aleph1]

Great! I think it is good to merge, but I am also willing to do some more experimentation if you would prefer. It might just take me a few days to get to it.

…

On Aug 5, 2023, at 7:33 AM, Fábio Santos ***@***.***> wrote:  Looks good to merge! I can handle the docs. I'm leaving it up to you whether you'd like to experiment with checking how often an import is used, before deciding whether to mangle, or if you think this is good to merge. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

[fabiosantoscode]

I'm not in any sort of rush. If you have a project with many imports, you could find out whether gzip will like this or not and benefit everyone in the process :) so you could say I would prefer it if you're up to it!