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

Adapt Assetic cookbook to not suggest now-deprecated YUI Compressor #1841

Closed
johnkary opened this issue Oct 22, 2012 · 9 comments
Closed

Adapt Assetic cookbook to not suggest now-deprecated YUI Compressor #1841

johnkary opened this issue Oct 22, 2012 · 9 comments
Labels

Comments

@johnkary
Copy link
Contributor

Referring to this cookbook article: How to Minify JavaScripts and Stylesheets with YUI Compressor

YUI recently announced on their blog that YUI Compressor will soon be going through a deprecation phase and the YUI team will be switching to using custom-wrapped versions of UglifyJS and CSSMin, called yuglify and ycssmin respectively.

So now what tools should we advocate readers to use for minification? Do we follow their lead and tailor to existing Assetic filters UglifyJSFilter and CSSMinFilter? Or even wrap their custom versions?

Once decided we can adapt the existing cookbook article and probably rename it to minify.rst to not tie it to a product.

@Sgoettschkes
Copy link
Contributor

I think it's a good idea to remove the Yui Compressor from the cookbooks as it is deprecated. ember.js for example doesn't work with the yui compressor anymore.

That said, uglifyjs has dependencies on node.js, meaning it will get complicated and would not be a good choice for a cookbook which introduces people to how assetics work. I really like the way yui compressor can be used (download, place inside project, done).

Regarding yuglify: If one is not compressing yui, there is no need to use the yuglify fork. uglify.js works well for most use cases.

Maybe it would be good to make a more generic cookbook, describing how it works and let the user choose which filter to actually use.

@johnkary
Copy link
Contributor Author

After some Twitter discussion with @kriswallsmith,@Seldaek and @stof, we came up with a few ideas.

Firstly, nobody objected to installing npm-based modules in a Symfony project. Kris thought UglifyJS would be an OK thing to advocate for in the Cookbook.

It would also require setting the assetic.node_paths config setting to include the path to the created node_modules directory.

Other discussion was a little more general, in how best to install npm modules to be packaged with a Symfony2 application. I'm looking for other feedback and discussion related to the install method:

Install to the app directory

Install the needed modules. This would result in a directory app/node_modules being created by npm that contains all the UglifyJS code and its dependencies.

cd app
npm install uglify-js

Then tell Assetic where to find the node modules we just added:

Set assetic.node_paths:

assetic:
    node_paths: [%kernel.root_dir%/node_modules]

Now here's where it gets interesting:

The NPM FAQ and his blog post node_modules in git both advocate for adding the node_modules directory to version control for "projects you deploy." Our use case falls under that; all Symfony SE-based applications intend to be deployed eventually :)

The blog post suggests checking in node_modules because npm does not lock dependencies deeper than the first level. So if UglifyJS depended on FooBuzzJS, the npm lock file would not lock FooBuzzJS to a specific commit. So next time someone tries to install UglifyJS (maybe later when doing a deployment), it might resolve to a slightly newer version of FooBuzzJS. This puzzled me for a second because I believe Composer locks all dependencies, so I assumed all dependency managers did this.

So in this case, we would advocate for using npm install to add dependencies like UglifyJS.

Install via Composer

If we ignored the advice of the NPM authors ⚠️ ... which we probably shouldn't do... but for the sake of discussion, we could install NPM modules via a Composer script after each composer install or composer update command.

@stof suggested this:

{
    "scripts": {
        "post-install-cmd": ["npm install"],
        "post-update-cmd": ["npm install"]
    }
}
  1. Is it possible to tell NPM to install inside Composer's vendor dir? If not, node_modules would need to be added to the project's .gitignore
  2. assetic.node_modules would also need to be set with the path to the node_modules directory.

@stof
Copy link
Member

stof commented Jan 10, 2013

@johnkary Actually, npm does not lock the dependencies at all. If your top-level dependency is on uglify-js: *, a later install can receive a newer version of uglify-js. Locking the dependencies is done manually.
To compare with composer, they have the composer.json but not the composer.lock.
and btw, Bower (frontend package manager) does not lock either (it works the same than npm in this regard).

@kriswallsmith
Copy link
Contributor

We should follow what the NPM authors suggest. My $.02…

@Seldaek
Copy link
Member

Seldaek commented Jan 11, 2013

@kriswallsmith as I understand it here, the only thing installed via node are some build tools, as such they should be used in dev and in build, but not really for production, so I don't know if the suggestion of the FAQ applies.

@Sgoettschkes
Copy link
Contributor

I'm also not sure if the way the npm modules should be installed is clear enough to mention in a cookbook. I for example have my vagrant box with uglifyjs installed globally, so no need to install it on a per-project basics. I am also against having tools inside the vcs, especially if they are not used for production (and I guess not many people will build their assets on a production system).

@Sgoettschkes
Copy link
Contributor

I made a first draft https://github.com/Sgoettschkes/symfony-docs/commit/d2ed2179d5f678c018bdd4a95c619f9171b33346. Please have a look and give feedback.

I chose to advocate installing the npm module globally and add a note about the local installation. I don't think we should go into details about npm and how to add it to composer, as it would complicate the issue.

If this really is an issue worth discussing in a cookbook, I would opt for creating a new cookbook where issues with npm modules (uglifyjs, coffeescript) and ruby gems (e.g. sass) are discussed.

@Sgoettschkes
Copy link
Contributor

Regarding #1099: If we move from yuicompressor to uglifyjs, there is no css involved anymore.

I made some small changes in https://github.com/Sgoettschkes/symfony-docs/commit/10fa57b21485b1a128bedb2c07037383afe64601, but need some more feedback if the direction I'm going is ok.

We should also think about how to minify/combine css because yui does this while uglify doesn't. I am always using sass, which can be used to minify and combine, so I don't have any idea what is used when working with vanilla css.

@wouterj
Copy link
Member

wouterj commented Mar 30, 2013

I think it's good to have one article like this which gives people a great usecase of assetic.

However, as stated here, we want to add a section with a list of all filters available. So, I don't think we need to worry much about which filters we are going to use in this article.

@wouterj wouterj closed this as completed Sep 8, 2014
weaverryan added a commit that referenced this issue Oct 1, 2014
…eguiluz)

This PR was merged into the 2.3 branch.

Discussion
----------

Added a note about the total deprecation of YUI

| Q             | A
| ------------- | ---
| Doc fix?      | no
| New docs?     | yes
| Applies to    | 2.3+
| Fixed tickets | -

Maybe we could also close the related issue #1841?

Commits
-------

f5021d5 Fixed the official name of Node.js
d027c80 Added a note about the total deprecation of YUI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants