Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Concatenation or some other method of importing? #5

Closed
rodaine opened this issue Aug 21, 2014 · 11 comments
Closed

Concatenation or some other method of importing? #5

rodaine opened this issue Aug 21, 2014 · 11 comments

Comments

@rodaine
Copy link
Contributor

rodaine commented Aug 21, 2014

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

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
Copy link
Member

parkr commented Aug 21, 2014

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 👍 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
Copy link
Member

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

Here's my original take: jekyll/jekyll#2770 (comment)

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 must place my JavaScript inside the _includes folder. That's gross. I'd rather _includes be kept for partials in layouts.
  • There's still no compression/minification.

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

  • Create a new tag, called asset_include (or relative_include). This would allow you to include files only within the same level or in a directory below the current file. This is essentially a jail and prevents you from include files from other sibling directories.
  • Add a new config.yml key, javascript, which can take compression options. Essentially, a wrapper for uglifier.

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. 😜 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 👎 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
Copy link
Member

parkr commented Aug 22, 2014

Whoa. http://duojs.org

@parkr
Copy link
Member

parkr commented Sep 7, 2014

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

@gjtorikian
Copy link
Member

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

@parkr
Copy link
Member

parkr commented Oct 6, 2014

Some of this will be satisfied by the include_relative tag in Jekyll v2.4.0. jekyll/jekyll#2870

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

@gjtorikian
Copy link
Member

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
Copy link
Member

parkr commented Oct 6, 2014

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

@gjtorikian
Copy link
Member

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 ❤️ the 💩 out of Pages, and are trying to make it friendlier for plugins. Zero guarantees or plans on any timeline whatsoever, though.

@parkr
Copy link
Member

parkr commented Oct 7, 2014

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

@jekyllbot
Copy link
Contributor

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.

@jekyllbot jekyllbot added the stale label Jun 6, 2016
soundasleep added a commit to soundasleep/jevon.org that referenced this issue Nov 15, 2018
NOTE that on GitHub pages, we can't run any sort of browserify or
node_modules importing, so there's no point in having a package.json.

See: jekyll/jekyll-coffeescript#5
@jekyll jekyll locked and limited conversation to collaborators Apr 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants