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

minify makes js bigger #15

Closed
sirenkovladd opened this issue Jun 5, 2024 · 13 comments
Closed

minify makes js bigger #15

sirenkovladd opened this issue Jun 5, 2024 · 13 comments
Labels
bug Something isn't working

Comments

@sirenkovladd
Copy link
Contributor

  • Version (npm -v): 10.5.2
  • Node Version node -v: v20.13.1
  • OS (uname -a on Linux): Darwin arm64

Looks like @putout/minify makes js bigger

import { minify } from "@putout/minify";

const source = `()=>{try{return 1}catch(e){return console.log(e),2}}`;
const d = minify(source);
console.log(d);

console.log(source.length, '->', d.length);
❯ node test.js
()=>{try {return 1} catch (e) {console.log(e);return 2}};
52 -> 57
@sirenkovladd
Copy link
Contributor Author

also

const source = `for (let e of l)(e.o=w(e.o)),(e.l=w(e.l))`;

result

for(let e of l){(e.o=w(e.o));(e.l=w(e.l))}

expect

for(let e of l)e.o=w(e.o),e.l=w(e.l)

@sirenkovladd
Copy link
Contributor Author

also

const source = `() => { d(); return 1 }`;

result

()=>{d();return 1};

expect

()=>(d(),1)

@coderaiser coderaiser added the bug Something isn't working label Jun 7, 2024
@coderaiser
Copy link
Contributor

Thanks! Current result:

()=>{try{return 1}catch(e){console.log(e);return 2}};

@coderaiser
Copy link
Contributor

Just updated, please try latest version of @putout/minify. Is it works for you?

@sirenkovladd
Copy link
Contributor Author

2/3 works

const source = `for (let e of l)(e.o=w(e.o)),(e.l=w(e.l))`;

does not work for this line

@coderaiser
Copy link
Contributor

Landed in v3.19 🎉.

Is it works for you?

@sirenkovladd
Copy link
Contributor Author

Almost, now I have it

for(let e of l)(e.o=w(e.o)),(e.l=w(e.l));

but expect

for(let e of l)e.o=w(e.o),e.l=w(e.l)

@coderaiser
Copy link
Contributor

Landed in v3.19.1 🎉. Is it works for you?

@sirenkovladd
Copy link
Contributor Author

image
It looks like not

@coderaiser coderaiser reopened this Jun 9, 2024
@sirenkovladd
Copy link
Contributor Author

Also found a related error

const source = `export const s = () => a ? (p(),i) : 2`

result

export var s=()=>a?p(),i:2;
export var s=()=>a?p(),i:2;
                      ^

SyntaxError: Unexpected token ','
    at ModuleLoader.moduleStrategy (node:internal/modules/esm/translators:169:18)
    at callTranslator (node:internal/modules/esm/loader:272:14)
    at ModuleLoader.moduleProvider (node:internal/modules/esm/loader:278:30)
    at async link (node:internal/modules/esm/module_job:78:21)

Node.js v20.13.1

@coderaiser
Copy link
Contributor

Could you please create another issue for another cases

@coderaiser
Copy link
Contributor

Landed in v3.19.3.

SequenceExpressions is black hole I would be happy to add more features to support every case, but it will be definitely a lot of them, maybe 20 more, who knows, let's use different issues for each syntax related case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants