config doesn't understand symbol values

[cykerway]

jekyll-assets/lib/jekyll/assets/compressors/uglify.rb

Line 22 in 4e614e8

config = e.asset_config[:compressors][:uglifier].symbolize_keys

Cannot set an option to tell uglifier to only keep copyright comments in _config.yml.

Doesn't work:

compressors:
    uglifier:
        comments: copyright

Doesn't work:

compressors:
    uglifier:
        comments: :copyright

uglifier is expecting a symbol. However, it seems whatever you write in jekyll _config.yml it's passed as a string. Have to modify the code here to handle a string value instead of simply symbolizing the keys. Plus, jekyll-assets sets default comment option to false. However, if you don't set it, the default in uglifier is :copyright, which may be more useful.

[cykerway]

To add some details, you can use a symbol in config.rb:

compressors: {
    uglifier: {
        comments: :copyright,
        harmony: Utils.new_uglifier?,
    },
},

[envygeeks]

I don't know that I want to go digging through Uglifiers options to flag what should be a symbol, and not be a symbol, I do know this is a Jekyll bug, as they're the ones that are turning your symbols into Strings in the name of safety (even though Ruby has had a symbol GC for quite some time) and maybe Uglifier could be more relaxed (considering it must in some way pass that to JavaScript which doesn't have the concept of a Ruby symbol (though it does have Symbols now), but that's neither here-nor there in this context.) I would vote take this to Uglifier and see if they can relax a bit, but it's not really their responsibility as much as it is Jekyll's.

I'll sit on it for a few days and decide what to do, in the meantime, maybe you could consider taking this bug to Jekyll, and maybe even seeing what Uglifier has to say about it.

[cykerway]

I forgot where I saw it but it seems Jekyll is aware of the problem of not supporting symbols in the _config.yml file. But it's YAML so probably nobody's expecting a ruby symbol there.

Accepting a ruby symbol in option is the design choice of uglifier (in ruby). Not sure this is good or not but they have decided to do that. Since jekyll-assets is a downstream of uglifier, I think it should pass whatever arguments uglifier expects. If a downstream decides no to follow the upstream's convention, then this should be explicitly listed to not confuse users. However, in this case, I think the problem is not hard to fix. This is what uglifier expects for comment options:

def comment_options
  case comment_setting
  when :all, true
    true
  when :jsdoc
    "jsdoc"
  when :copyright
    encode_regexp(/(^!)|Copyright/i)
  when Regexp
    encode_regexp(comment_setting)
  else
    false
  end
end

There are only 5 cases in which you need to detect a string and convert it to what uglifier expects.

These being said, as you said above if uglifier is relaxed a bit (taking a string) then maybe there doesn't have to be a change here in jekyll-assets.

[cykerway]

One more thing: I think you can add a hotfix by using :copyright as default in config.rb. If users really want it to be false, then they can easily modify their _config.yml to set it to false. If you read the function comment_options above, you will see even though ":copyright" doesn't give rise to :copyright, "false" does give rise to false. So users simply need to use a string value in their _config.yml file to set it to false.

[envygeeks]

YAML so probably nobody's expecting a ruby symbol there.

YAML was designed explicitly for this type of data. It's a serialization, and data interchange format. If Jekyll wanted only strings they should have opted for JSON.

Since jekyll-assets is a downstream of uglifier, I think it should pass whatever arguments uglifier expects.

Then take it up with Jekyll, I'm not in the habit of fixing upstreams bugs. One of my upstreams is causing a bug with another upstream, that doesn't put the responsibility in my hands, and if I do assume responsibility... all it does is make my software less performant, and my job harder when I have to track specific bugs at specific places, so that I can walk back a fix when and if it's ever fixed upstream, or just leave my software with dead and worthless code. I'm not funded by Github, or have Github employees helping me, like Jekyll.

If a downstream decides no to follow the upstream's convention, then this should be explicitly listed to not confuse users.

Again, you are putting the responsibility for a bug Jekyll has created, in my hands. This is not a good way to convince me to work around this problem.

[cykerway]

All right. I just read the issue you posted on jekyll: jekyll/jekyll#6558 So at least we understand the situation is uglifier expects a symbol while jekyll uses a string and jekyll-assets, sitting in the middle, doesn't want to tamper with the options.

However, I want to point out that the problem here is a bit different than the one you listed, in that it's not really about symbolized keys, but values.

Now that the situation is clear and you are aware of it, you may choose to close this thread if you want. But the problem remains: uglifier -> jekyll-assets -> jekyll, at least one needs to change for things to work.

[envygeeks]

This can be fixed in 4.x given we have 2.6 as a requirement now and there is a symbol GC.