jekyll / jekyll-coffeescript

A CoffeeScript converter for Jekyll.
MIT License
53 stars 20 forks source link

Concatenation or some other method of importing? #5

Closed rodaine closed 8 years ago

rodaine commented 10 years ago

While Sass comes with @importing out of the box, JS and CoffeeScript lack that functionality. Currently we have patterns like CommonJS, AMD, and (soon-to-be) ES6 Modules for managing our dependencies with browser-side scripts and compiling them into a single, optimized file for production. However, this necessitates build tools and - at least with Github Pages - requires committing these compiled files. :scream:

So, I was thinking of forking this bad boy to potentially add some sort of support for any or all of the patterns I've suggested (Vanilla concatenation, CommonJS, AMD, or ES6 Modules), but wanted some input first on which would be most appropriate. I know there is some limitations with supporting plugins up on Github Pages, so I figured I'd ask.

Thoughts?

parkr commented 10 years ago

This is a great idea.

I think we should stick with just one idea though, and preferably something that would work in either JS or CoffeeScript. We have Liquid at our disposal already so if that can be wrangled to work with this, then :+1: Which method do you think is best?

In safe mode, nothing outside the site's source should be accessible, and no symlinks are allowed. That's pretty much it in terms of security. Sites on GHP must be self-contained.

gjtorikian commented 10 years ago

I missed this issue, but the compression sentiment is in the air!

Here's my original take: https://github.com/jekyll/jekyll/issues/2770#issuecomment-52985097

Here's the same text produced below (so you don't have to read it elsewhere):

There's an existing Jekyll/Liquid idiom that looks like this: create an assets file called application.js, and trick it into thinking it's important by marking with an empty ---:

---
---

{% include assets/javascript/jquery.min.js %}
{% include assets/javascript/foo.js %}
{% include assets/javascript/whatever.js %}

If you place jquery.min.js, foo.js, and whatever.js into your _includes directory, it'll combine them into one file. This also picks up CoffeeScript, provided it also has the empty ---.

Two glaring problems:

I propose making a change to Jekyll to do the following:

I can do the legwork to get a PR going if there's interest. Otherwise, I'll take my ideas elsewhere...and put them in a gem. :stuck_out_tongue_winking_eye: My only hesitation with the gem approach is not providing this for GitHub Pages users. But maybe we can whitelist it down the road. shrug


I'm :-1: on anything that requires require, because it means the site owner may need to redo some of their custom JS, or otherwise be at the mercy of figuring out some zany workaround. The thing I appreciate about Jekyll is that it's stupid easy for people to get things done. So I opt for the most vanilla process possible.

parkr commented 10 years ago

Whoa. http://duojs.org

parkr commented 10 years ago

@rodaine What do you think of https://github.com/jekyll/jekyll/pull/2870? It won't minify, but it'll concat nicely, without weird nesting in _includes.

gjtorikian commented 10 years ago

For dead simple minification, there's https://github.com/gjtorikian/jekyll-jsminify

parkr commented 10 years ago

Some of this will be satisfied by the include_relative tag in Jekyll v2.4.0. https://github.com/jekyll/jekyll/pull/2870

Just minification is left, right? Maybe a Minify converter? Conversion is a chained thing now.

gjtorikian commented 10 years ago

Internally, we are using https://github.com/gjtorikian/jekyll-jsminify. But we couldn't find a secure and safe way of exposing this plugin to everyone, so we had to punt on that for now.

parkr commented 10 years ago

@gjtorikian are you able to expound on the security concerns?

gjtorikian commented 10 years ago

Mostly, there's an Uglifier option called evaluate, which asks Uglifier to evaluate constant expressions. It literally uses eval. We don't know what sorts of crazy JS exists in the world to permit eval to run. But the idea of "force-overriding" this setting to false has come up as a possible workaround.

As you know, the security team is rather small and super overworked. Even if a hundred people came out and said "the minifier poses no security threat," it still needs to be reviewed. But we all :heart: the :hankey: out of Pages, and are trying to make it friendlier for plugins. Zero guarantees or plans on any timeline whatsoever, though.

parkr commented 10 years ago

@gjtorikian Totally, totally. I don't mean to put any blame on you guys; y'all have been the most accommodating bunch around, and I have mad :cupid: for that. I was just curious about what the problem was to see if I could maybe fix it. Force-overriding the evaluate option to false seems reasonable to me. :+1:

jekyllbot commented 8 years ago

This issue has been automatically marked as stale because it has not been commented on for at least three months.

The resources of the Jekyll team are limited, and so we are asking for your help.

If you can still reproduce this error on the

3.1-stable
or
master
branch, please reply with all of the information you have about it in order to keep the issue open.

If this is a feature request, please consider building it first as a plugin. Jekyll 3 introduced hooks which provide convenient access points throughout the Jekyll build pipeline whereby most needs can be fulfilled. If this is something that cannot be built as a plugin, then please provide more information about why in order to keep this issue open.

Thank you for all your contributions.