douglascrockford / JSMin

JavaScript Minification Filter
http://javascript.crockford.com/jsmin.html
695 stars 151 forks source link

JSmin removed spaces after literals in strings. #22

Closed mikenewbon closed 2 years ago

mikenewbon commented 3 years ago

Currently when using a litteral in a string, for example in a query selector, the spaces are removed after the }.

document.querySelector(`${id} .read-more-elip`).classList.toggle('d-none'); complies to document.querySelector(`${id}.read-more-elip`).classList.toggle('d-none'); which breaks the querySelector.

Lazerbeak12345 commented 3 years ago

I didn't realize that #23 was a dupe of this till now, but I have more insight as to what the problem is, and a workaround there, if you are interested.

louking commented 2 years ago

I'm seeing the same problem, just came looking for this here.

          d3.select(`.tablerow-${year} .date`).text(`${formatDate(d.date)}/${year}`);
          d3.select(`.tablerow-${year} .count`).text(d.count);

is getting translated to

d3.select(`.tablerow-${year}.date`).text(`${formatDate(d.date)}/${year}`);d3.select(`.tablerow-${year}.count`).text(d.count);

Note the spaces missing before .date and .count in the two statements

louking commented 2 years ago

I bumped into this again. Is this repo abandoned?

If anyone finds this I decided to use rjsmin, which comes included with webassets (see https://webassets.readthedocs.io/en/latest/builtin_filters.html#webassets.filter.rjsmin.RJSMin). rjsmin does not have this issue.

thradams commented 2 years ago

I minified this input

d3.select(.tablerow-${year} .date).text(${formatDate(d.date)}/${year}); d3.select(.tablerow-${year} .count).text(d.count);

and I got

d3.select(.tablerow-${year} .date).text(${formatDate(d.date)}/${year});d3.select(.tablerow-${year} .count).text(d.count);

The character ` is processed like " and '

Maybe the encode (code page) you are using?

On Fri, 22 Apr 2022 at 06:57, Lou King @.***> wrote:

I bumped into this again. Is this repo abandoned?

If anyone finds this I decided to use rjsmin, which comes included with webassets (see https://webassets.readthedocs.io/en/latest/builtin_filters.html#webassets.filter.rjsmin.RJSMin). rjsmin does not have this issue.

— Reply to this email directly, view it on GitHub https://github.com/douglascrockford/JSMin/issues/22#issuecomment-1106303158, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSMZLTIDFS7TANBOVTIHTTVGJZXRANCNFSM4ZGXM2JQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- www.thradams.com

louking commented 2 years ago

Maybe the encode (code page) you are using?

Not sure what this means. My compressed file (on my windows development system) is coming up as UTF-8 from jsmin (and rjsmin, which compresses properly). I see the same issue in my production system which is CentOS 7. I can check the encoding there if that seems important.

I'm using jsmin-3.0.1. I'm running it via flask-assets 2.0 using webassets 2.0.

What else could be affecting this behavior?

thradams commented 2 years ago

Can you attach one small input file that fails for you? Then I can debug.

On 22 Apr 2022, at 09:38, Lou King @.***> wrote:

 Maybe the encode (code page) you are using?

Not sure what this means. My compressed file (on my windows development system) is coming up as UTF-8 from jsmin (and rjsmin, which compresses properly). I see the same issue in my production system which is CentOS 7. I can check the encoding there if that seems important.

I'm using jsmin-3.0.1. I'm running it via flask-assets 2.0 using webassets 2.0.

What else could be affecting this behavior?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.

louking commented 2 years ago

I'll see if I can come up with something -- may be a couple of days. Thanks.

If "not small" is ok, you can use this one.

charts.zip

e.g., https://github.com/louking/loutilities/blob/a1a2789c815b077fb08fe58a23d0db3a0337090f/loutilities/tables-assets/static/charts.js#L392 is being compressed as d3.selectAll(`${that.options.containerselect}.focuslabel-${label}`).style("display",null);

louking commented 2 years ago

I edited the previous comment and want to make sure you saw my edits. Please let me know if you need me to come up with a smaller example.

louking commented 2 years ago

It turns out it was pretty easy to recreate in a small example. In my environment (flask-assets 2.0; webassets 2.0; jsmin 3.0.1), test.js gets compiled by jsmin into testjs.js, in the enclosed zip file.

douglascrockford-JSMin#22.zip

asset_bundles = {
    'test_js': Bundle(
        'test.js',

        filters='jsmin',
        output='gen/testjs.js',
    ),

   ...
thradams commented 2 years ago

I edited the previous comment and want to make sure you saw my edits. Please let me know if you need me to come up with a smaller example.

Using your file I got: d3.selectAll(${that.options.containerselect} .focuslabel-${label}).style("display",null);

Are you sure you are using the latest version?

louking commented 2 years ago

I installed jsmin 3.0.1 from pypi. At least that is what pip freeze tells me. Is there something later than that?

From my requirements.txt file:

jsmin==3.0.1

louking commented 2 years ago

I installed this as a pip upgrade, I think from jsmin 2.2.2 (not sure because I didn't capture the original install in the log I keep -- grr). Is it possible that the old executable is being used somehow in that use case? Just grasping at straws here...

louking commented 2 years ago

Many apologies. Looking at pypi I see now that the correct repo is https://github.com/tikitu/jsmin/

I am not sure how I found your repo initially.

Sorry for any wasted effort.

douglascrockford commented 2 years ago

JSMin is written in C. You could have easily distinguished this project from your Python crap.

If it is any comfort to you, you are not the first to commit this blunder.