google / closure-compiler

A JavaScript checker and optimizer.
https://developers.google.com/closure/compiler/
Apache License 2.0
7.4k stars 1.15k forks source link

String.fromCharCode optimization #3606

Open kittenswolf opened 4 years ago

kittenswolf commented 4 years ago

I used: https://closure-compiler.appspot.com/home

Input code: document.write(String.fromCharCode(115));

Expected output: document.write("s")

Actual output: document.write(String.fromCharCode(115));

I expected the String.fromCharCode function to be removed to just the resulting character.

brad4d commented 4 years ago

We'd certainly entertain a PR for this one, but it's unlikely to be a priority for us.

concavelenz commented 4 years ago

@kittenswolf Can you provide a usecase where this would be valuable?

aadi-thedevguy commented 2 years ago

Hey, I'm new to open source and this is my first project. I have a good 1 year experience with javascript.

so if no one's working on this , I would love to help. Hoping to hear from anyone regarding this.

lauraharker commented 2 years ago

@monztercoder I'll assign you to this issue and we'd be happy to review a PR. Like @concavelenz mentioned earlier, we don't have a compelling use case for this optimization yet, so I can't promise we'll be able to accept that PR.

This optimization should go in https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/PeepholeReplaceKnownMethods.java.

adhamahmad commented 1 year ago

I am new to open source and would like to work on this If this is still open @lauraharker

lauraharker commented 1 year ago

Hi @adhamahmad, this is still an open issue and you're welcome to look into it.

As mentioned in previous comments, we will review a PR for this but can't guarantee we will accept it. (there's a tradeoff between the maintenance burden of additional optimizations & the potential code size improvement)

adhamahmad commented 1 year ago

I am having trouble building the jar file of the compiler.

When I run the following command bazelisk build //:compiler_uberjar_deploy.jar

I get the following error: image

Any help on how to set up the development environment would be apprciated. @lauraharker

niloc132 commented 1 year ago

I can't reproduce a failure on linux at least - perhaps this is something to do with running on windows in the cmd shell. Can you try WSL2 instead, so that you have a working *nix environment? It seems common to write bazel rules as shell commands, which then assumes that every user has the exact same implementations of those commands (e.g. no differences in find or zip behavior in that command in your screenshot).

Very likely rewriting that shell command for :externs_zip into something that works cross platform (starlark, etc) would solve this.

adhamahmad commented 1 year ago

Thanks for your concern. I tried WSL2 (Ubuntu) and still got the same error. @niloc132

niloc132 commented 1 year ago

You might try running the command directly, and seeing what the error is then?

Also, I can't recall exactly where the dividing line is between windows and wsl, but it might be important to run bazelisk shutdown before switching to a new shell, to ensure that the daemon is stopped, new details picked up properly?