closure-compiler icon indicating copy to clipboard operation
closure-compiler copied to clipboard

String.fromCharCode optimization

Open kittenswolf opened this issue 4 years ago • 10 comments

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.

kittenswolf avatar May 27 '20 15:05 kittenswolf

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

brad4d avatar May 27 '20 22:05 brad4d

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

concavelenz avatar Jun 22 '20 17:06 concavelenz

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.

aadi-thedevguy avatar Jan 23 '22 17:01 aadi-thedevguy

@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.

lauraharker avatar Jan 24 '22 18:01 lauraharker

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

adhamahmad avatar Jul 25 '23 03:07 adhamahmad

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)

lauraharker avatar Jul 28 '23 20:07 lauraharker

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

adhamahmad avatar Jul 29 '23 03:07 adhamahmad

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.

niloc132 avatar Jul 30 '23 18:07 niloc132

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

adhamahmad avatar Jul 30 '23 18:07 adhamahmad

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?

niloc132 avatar Jul 30 '23 19:07 niloc132