It's not working for me. #49

nirsharma · 2016-10-09T19:55:51Z

this is my code
app.use(minifying({ js_match : /\.js$/, css_match : /\.css$/ })) right before express static middleware.

no js or css file is being minified.
not getting any response to data APIs

I tried using compression, that works just fine. But minification , with or without compression is not working.

The text was updated successfully, but these errors were encountered:

breezewish · 2016-10-11T06:12:22Z

Hi, could you try with just app.use(minifying())?
js_match and css_match are matched with content-types, instead of urls.

nirsharma · 2016-10-11T06:54:51Z

I had tried with just app.use(minifying()) but it didn't work, files weren't loading

tayler-king · 2016-10-11T18:04:29Z

+1

Using App.use(require('express-minify')()); - not minifying any css or javascript files for me either.

Martii · 2016-10-11T18:34:57Z

It's working on our production vs. development... has anyone noticed that minifying should probably be minify ?

Also what version is your express?

npm update tried?

Is there any package in your project that modifies express's MIME content types from their standards? e.g. express.static.mime.define...

Ref(s):

Our current declaration code point
Our current activation code point
Ordering is important with express and interaction with some other middle-ware too.

nirsharma · 2016-10-11T19:10:58Z

@Martii

const minifying = require('express-minify') so it shouldn't be a problem.
express version is 4.14.0
not even touching the MIME content types
I am using it along with compression middleware which is working fine but not this one.

tayler-king · 2016-10-11T19:30:37Z

@nnIIInnjA Same with me, apart from not using the compression middleware.

I've tried using the middleware in every order combination possible - still no success.

Edit: Just took a look, and it seems to be minifying some files (some JS files), but not others (some JS files, CSS files). It's not showing any errors either.

Okay, seems to be an issue of files failing to minify but the onerror function not being called.

This file, for example, will not minify nor will it error:

var Routes = angular.module('routes', []);

Routes.config(['$routeProvider', function($RouteProvider){

    var $Route = $RouteProvider.$get[$RouteProvider.$get.length-1]({$on:function(){}});

    $RouteProvider
    .when('/duel', {
        templateUrl: 'partials/duel',
        controller: 'Duel',
        reloadOnSearch: false
    })
    .otherwise({
        redirectTo: '/duel'
    });

}]);

I've also found that using let (ES6/ES2015) will result in SyntaxError: Unexpected token: name ([Variable Name]) - I had to replace all let declarations with var.

UglifyJS doesn't officially support ES6/ES2016 - see mishoo/UglifyJS#448

Also just noticed, CSS files won't be minified if you have an @import statement in the CSS.

Although these aren't directly issues with this library, there should (from my point of view) have errors occur if these tasks fail.

Martii · 2016-10-12T01:54:40Z

@KonDax

UglifyJS doesn't officially support ES6/ES2016

That's why OUJS has forked the harmony branch to coincide with release that express-minify here uses. See our fork at https://github.com/OpenUserJs/UglifyJS2/tree/harmony%2Brenamed .

Also just noticed, CSS files won't be minified if you have an @import statement in the CSS.

That's #18 here.

I am using it along with compression middleware which is working fine but not this one.

We are too although I haven't checked the headers recently to ensure Gzippin is occuring... See https://github.com/OpenUserJs/OpenUserJS.org/blob/98c5b3f1ddbff33e2ab7e11052d97b894e4d1946/app.js#L272

EDIT: Retested PASS

GET /css/common.css HTTP/1.1
Host: openuserjs.org
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:43.0) Gecko/20100101 SeaMonkey/2.40
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
DNT: 1
Connection: keep-alive

HTTP/1.1 200 OK
X-Powered-By: Express
Strict-Transport-Security: max-age=31536000000; includeSubDomains
Accept-Ranges: bytes
Cache-Control: public, max-age=86400
Last-Modified: Wed, 31 Aug 2016 19:46:17 GMT
Etag: W/"159e-156e2234328"
Content-Type: text/css; charset=UTF-8
Vary: Accept-Encoding
Content-Encoding: gzip
Date: Wed, 12 Oct 2016 02:02:25 GMT
Connection: keep-alive
Transfer-Encoding: chunked

In general it's quite picky getting the order right and we rarely mess with it. I just retested this yesterday because of this issue too. I'm usually watching for potential issues for our usage as well.

EDIT

there should (from my point of view) have errors occur if these tasks fail.

uglifyjs2 is the one that throws the errors not express-minify... they are just caught here usually.

@nnIIInnjA

const minifying...

Have you tried var... perhaps there's a node issue with ES6 let and const... don't know for sure... we are primarly ES5 with the minification as experimental right now with Userscripts since uglifyjs2 ES6 support is on a branch and not release... however our actual production is using release uglifyjs2.

breezewish · 2016-10-12T02:12:14Z

Yeah. The default error behavior is to return the original content (so that errors won't break anything) when there is a minifying error. https://github.com/breeswish/express-minify#customize-onerror-behavior

Since more and more people is using ECMAScript 2016+ which is not supported by uglifyjs currently, I would update the documentation to suggest users to add an error handling logic so that they will get noticed about this issue.

Martii · 2016-10-12T02:17:32Z

..add an error handling logic...

Yep that's here in ours from #44 here... Ace editor (ace-builds) sometimes, but not all the time, does throw one with release uglifyjs2. I've been watching that project too... haven't pinned down what Ace is doing and how to reproduce yet but it's only about twice a month.

nirsharma · 2016-10-12T07:28:40Z

@Martii I tried with var too. not working :(
FYI m not using any import statement in my css, it's very basic plan css file. Also js code is transpiled down to es5 before server to fetch so that also shouldn't be a problem

tayler-king · 2016-10-12T07:36:17Z

@Martii ah right, makes sense. Maybe an issue will have to be opened for UglifyJS2 not throwing errors correctly.

Well, at least everything apart from CSS seems to be minified for the most for me. CSS randomly works or not - but that library is super old.

Martii · 2016-10-12T08:03:41Z

@nnIIInnjA
Is your transpiler middleware too? e.g. what is it on npmjs.com and I can try to give it a test whirl in our development. We're using the same version of express as you... we're also still on node 4.x latest but development here on my dev station is node@6.x and it still returns the same results when I kick dev into ignoring dev vs pro and debug modes. Better question is your project open source so I can try it out with a git checkout?

EDIT
Do you get any errors with $ npm install btw? Any binary compiling issues?

@KonDax

Maybe an issue will have to be opened for UglifyJS2 not throwing errors correctly.

They currently allow "downstream" or "across the pond" to handle errors... express-minify catches them and allows additional processing to maintain site integrity... you wouldn't want a site just erroring out and having to restart the process_(es)_. Proposing that would be a breaking change in many, many projects that utilize uglifyjs2 though... not to mention the CLI. I linked in our error handler to show how to put in a handler for anyone watching including you... I may bump our body value up to 200 characters as it took me a few months to track down that Ace had an intermittent glitch on occasion but if it doesn't minify then that's why I'm over there to report issues. We have release (master) and harmony packages/git in order to help improve it... anyhow... it's up to you but breaking changes can be an issue in itself over there. They seem very receptive and I know how long development really takes especially in a modular system like node/npm.

but that library is super old.

Which library now?

nirsharma · 2016-10-12T08:07:37Z

@Martii Sadly my project isn't open source and nope transpiling is done beforehand and not in express.
I've also checked error while npm install none
this is the code :-
const h2 = require('http2')
const fs = require('fs')
const config = require('config')
const path = require('path')
const httpProxy = require('http-proxy')
const express = require('express')
const R = require('ramda')
const request = require('request')
const cookieParser = require('cookie-parser')
const gZipping = require('compression')
const minify = require('express-minify')
const app = express()
app.use(cookieParser())

express.request.__proto__ = h2.IncomingMessage.prototype;
express.response.__proto__ = h2.ServerResponse.prototype;

// to gZip files
app.use(gZipping())

// to minify files
app.use(minify())

// to serve static files
app.use('/', express.static(__dirname))

there's more code here as well but that is mostly to handle data apis and nothing to do with files

nirsharma · 2016-10-12T08:30:27Z

@Martii
Update :
I've tried hitting just to get the styles.css file and i got this :-
Resource interpreted as Document but transferred with MIME type text/css: "https://localhost:5555/styles.css".

Martii · 2016-10-12T08:38:53Z

@nnIIInnjA
After I built a basic package.json for your requires... and added in a listening port of 3000 at the tail of app.js... it's minifying our common.css copy here in node@6.x. :\

package.json

{
  "name": "Test",
  "description": "Test",
  "version": "0.0.0",
  "main": "app",
  "dependencies": {
    "http2": "3.3.6",
    "config": "1.21.0",
    "http-proxy": "1.15.1",
    "express": "4.14.0",
    "ramda": "0.22.1",
    "request": "2.75.0",
    "cookie-parser": "1.4.3",
    "compression": "1.6.2",
    "express-minify": "0.2.0"
  },
  "repository": {
    "type": "git",
    "url": "https://github.com/nnIIInnjA/test"
  },
  "license": "(GPL-3.0 AND GFDL-1.3)"
}

app.js

const h2 = require('http2')
const fs = require('fs')
const config = require('config')
const path = require('path')
const httpProxy = require('http-proxy')
const express = require('express')
const R = require('ramda')
const request = require('request')
const cookieParser = require('cookie-parser')
const gZipping = require('compression')
const minify = require('express-minify')
const app = express()
app.use(cookieParser())

express.request.proto = h2.IncomingMessage.prototype;
express.response.proto = h2.ServerResponse.prototype;



// to gZip files
app.use(gZipping());

// to minify files
app.use(minify());

// to serve static files
app.use('/', express.static(__dirname));


app.listen(3000);

http://localhost:3000/common.css

Only thing that I can think of is maybe your platform and or node is out of date. Did you build your own node or use someones prepackage? What is your platform btw?

nirsharma · 2016-10-12T08:40:46Z

@Martii
node version :- 6.6.0
MacOs Sierra

Martii · 2016-10-12T08:42:05Z

OS X is the one platform I haven't tried our project on... what is your current Xcode version? Linux here.

nirsharma · 2016-10-12T08:46:44Z

@Martii I haven't installed xcode. I don't see how that's relevant to our issue.
OS version :- macOS Sierra 10.12

Martii · 2016-10-12T08:48:48Z

@nnIIInnjA

I haven't installed xcode. I don't see why that's relevant to our issue.

If you build node it is. Didn't get an answer if you prepackaged or not. They are on 8 on El Capitan... and it's quite relevant as there many issues. I'm not ready to upgrade my Mac to Sierra yet for a month or more as Apple has too many issues with Xcode right now in El Capitan. Anyhow... your snippet works on Linux so I'm not entirely sure what the deal is.

nirsharma · 2016-10-12T08:50:42Z

@Martii ohh sorry, i missed there. I am not building node, it's prepackaged.

Martii · 2016-10-12T09:03:32Z

@nnIIInnjA
Homebrew (which says 6.2.1 here on my Mac but it's El Capitan build) or direct image from node ? You are one version behind the latest release at nodejs.. I'm at 6.7.0 on Linux.

Resource interpreted as Document but transferred with MIME type text/css: "https://localhost:5555/styles.css".

With this your MIME types might be foo'd or set incorrectly... http://www.rubicode.com/Software/RCDefaultApp/ , or equivalent, should at least be able to tell you if the OS itself is set correctly with those MIME types but that doesn't exclude if node is built incorrectly or for an older OS X.

nirsharma · 2016-10-12T09:12:13Z

@Martii i've had older version of node sometime back 6.1.0 which i installed using direct image of node. recently i updated to 6.6.0 using brew.

I am using http2 module. would that be an issue ?

Martii · 2016-10-12T09:29:26Z

@nnIIInnjA
Okay after I installed brews version onto El Capitan and using the coded snippets above it is minifying our common.css.

Sooooooooooo... at least El Capitan is confirmed to work with node@6.2.1 and express-minify@0.2.0 here. My guess it's something in Sierra and/or the built package of node on brew (they call them bottles here on my Mac)... I would suggest investigating if your 6.6.0 is for sure Sierra compatible and make sure OS X is up to date. I also have the "Command Line Tools" from the App store installed along with Xcode for another project.

nirsharma · 2016-10-12T09:30:35Z

@Martii I just tried with http1 , it seems to work fine. I think the issue is with http2 which i am using in my project

tayler-king · 2016-10-12T14:30:36Z

@Martii In regards to error handling, I mean if something for some reason fails to minify. I dumped your error handler into my application to see if it would help - still only errors randomly.

CSSMin is the library that seems rather outdated - no updates for a few years. That's what randomly works for minifying my CSS.

Martii · 2016-10-12T20:48:35Z

@KonDax

CSSMin is the library that seems rather outdated

Well if you see an issue might want to file it over there. With as fast as everything changes sometimes it's good to be reminded if something isn't working in a dependency.

still only errors randomly

If you can pin it down to a particular rule set file a bug report on his site. :) All of our CSS seems to be working and never had a CSS glitch in our logs yet... just JavaScript every once in a while for production. We even use LESS compiling to CSS and that's been good too.

tayler-king · 2016-10-12T21:13:19Z

@Martii well, when I say random I honestly mean it. Without changing any of my application or the files to be minified, sometimes they will be minified and sometimes not. No idea how to track the cause of that.

tayler-king · 2016-10-15T22:38:11Z

Still seems to be icky for me - not minifying certain files, not erroring either. If I minify the files directly through uglifyjs2, it works. Weird.

melroy89 · 2016-11-03T16:31:09Z

Use minify, and use var instead of const? Did you set the static folder, the folder where the js/css files should be located, to in my case the 'public' folder? Keep it simple first.

var minify = require('express-minify');
app.use(minify());
app.use(express.static(path.join(__dirname, 'public')));

I'm using express-minify 0.2.0 and expressjs version 4.13.4, all working here.

tayler-king · 2016-11-03T17:54:17Z

Not if I named the variable minifying.
On Thu, 3 Nov 2016 at 4:31 pm, Melroy van den Berg notifications@github.com
wrote:

Wait wait, you use minify_ing_?
The command is without 'ing':

app.use(minify());

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#49 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AQFo-nWOU54AeNff29KuA0ToaYwK8FSIks5q6gxNgaJpZM4KSF68
.

melroy89 · 2016-11-03T20:01:05Z

Noticed, I changed my comment view the webpage plz

On Nov 3, 2016 18:54, "Tayler King" notifications@github.com wrote:

Not if I named the variable minifying.
On Thu, 3 Nov 2016 at 4:31 pm, Melroy van den Berg <
notifications@github.com>
wrote:

Wait wait, you use minify_ing_?
The command is without 'ing':

app.use(minify());

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#49
issuecomment-258196613>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AQFo-
nWOU54AeNff29KuA0ToaYwK8FSIks5q6gxNgaJpZM4KSF68>
.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#49 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAmYvnFPNU40ZrkHCy9sI-RSDjjS36cBks5q6h_LgaJpZM4KSF68
.

tayler-king · 2016-11-03T20:17:26Z

Variable name doesn't change anything, nor does switching between the type of variable.

Static folder is of course set. I've switched to webpack anyhow now, since this is just annoying.

melroy89 · 2016-11-04T13:08:09Z

I understand, strange I can't not reproduce the issue yet. Since in my case it just works.. Could it be owner/privileges rights problem? Read & write.

tayler-king · 2016-11-04T13:12:10Z

Nope, has all the privileges needed.
On Fri, 4 Nov 2016 at 1:10 pm, Melroy van den Berg notifications@github.com
wrote:

I understand, strange I can't not reproduce the issue yet. Since in my
case it just works.. Could it be owner/privileges rights problem? Read &
write.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#49 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AQFo-ivjENHx2fC4M9xTO1Suut6hz9_Nks5q6y45gaJpZM4KSF68
.

breezewish · 2017-02-03T02:16:34Z

I'm going to add some debug output to ease the debugging.

martinlevesque · 2017-06-08T16:12:31Z

js/css were not minifying also for me. I have an nginx serving my node application and static files were served by nginx:

location ~ \.(html|js|css|gif|jpg|png)$ {
   root ...
}

To make js/css minified I changed it to

location ~ \.(html|gif|jpg|png)$ {
   root ...
}

breezewish · 2017-06-10T14:51:03Z

@martinlevesque Yes you have to use express to serve static files otherwise it won't work.

breezewish · 2017-09-01T17:34:05Z

Closed due to no activity for long time. Please open another PR if you still meet issues.

searene · 2018-07-17T14:30:17Z

One of my js files were not minified, either. After some experiments, I found out that the reason why it was not minified was that it used arrow functions. It worked after I changed it to a normal function.

Martii · 2018-07-17T20:13:56Z

@searene Cc: @breeswish

I found out that the reason why it was not minified was that it used arrow functions

Bad news ... ES5 is the only suppport that uglifyjs gives now and they have abandoned their harmony branch to concentrate only on older versions of ECMAScript (JavaScript).
Good news... ES6+ is being continued at /fabiosantoscode/terser and should offer feature parity for this project here. So you may utilize the uglifyJsModule option if you wish in this project.

It worked after I changed it to a normal function.

From experience the arrow operator doesn't offer any perceived enhancement and actually is a watered down version of function as you figured out.

LSafer · 2020-11-18T10:19:26Z

How could I utilize the uglifyJSModule option ? Should I make a custom implementation? Or is there a reference that I should pass as an option. Since the express-minify rejected mapping the option uglifyJSModule to the exported object from terser

breezewish added the enhancement label Oct 12, 2016

breezewish added the NFR label Feb 3, 2017

breezewish closed this as completed Sep 1, 2017

It's not working for me. #49

It's not working for me. #49

Comments

nirsharma commented Oct 9, 2016 • edited Loading

breezewish commented Oct 11, 2016

nirsharma commented Oct 11, 2016 • edited Loading

tayler-king commented Oct 11, 2016

Martii commented Oct 11, 2016 • edited Loading

nirsharma commented Oct 11, 2016

tayler-king commented Oct 11, 2016 • edited Loading

Martii commented Oct 12, 2016 • edited Loading

breezewish commented Oct 12, 2016

Martii commented Oct 12, 2016

nirsharma commented Oct 12, 2016 • edited Loading

tayler-king commented Oct 12, 2016

Martii commented Oct 12, 2016 • edited Loading

nirsharma commented Oct 12, 2016 • edited Loading

nirsharma commented Oct 12, 2016

Martii commented Oct 12, 2016 • edited Loading

nirsharma commented Oct 12, 2016

Martii commented Oct 12, 2016

nirsharma commented Oct 12, 2016 • edited Loading

Martii commented Oct 12, 2016 • edited Loading

nirsharma commented Oct 12, 2016

Martii commented Oct 12, 2016 • edited Loading

nirsharma commented Oct 12, 2016 • edited Loading

Martii commented Oct 12, 2016 • edited Loading

nirsharma commented Oct 12, 2016

tayler-king commented Oct 12, 2016

Martii commented Oct 12, 2016

tayler-king commented Oct 12, 2016

tayler-king commented Oct 15, 2016

melroy89 commented Nov 3, 2016 • edited Loading

tayler-king commented Nov 3, 2016

melroy89 commented Nov 3, 2016

tayler-king commented Nov 3, 2016

melroy89 commented Nov 4, 2016

tayler-king commented Nov 4, 2016

breezewish commented Feb 3, 2017

martinlevesque commented Jun 8, 2017 • edited Loading

breezewish commented Jun 10, 2017

breezewish commented Sep 1, 2017 • edited Loading

searene commented Jul 17, 2018

Martii commented Jul 17, 2018

LSafer commented Nov 18, 2020

nirsharma commented Oct 9, 2016 •

edited

Loading

nirsharma commented Oct 11, 2016 •

edited

Loading

Martii commented Oct 11, 2016 •

edited

Loading

tayler-king commented Oct 11, 2016 •

edited

Loading

Martii commented Oct 12, 2016 •

edited

Loading

nirsharma commented Oct 12, 2016 •

edited

Loading

Martii commented Oct 12, 2016 •

edited

Loading

nirsharma commented Oct 12, 2016 •

edited

Loading

Martii commented Oct 12, 2016 •

edited

Loading

nirsharma commented Oct 12, 2016 •

edited

Loading

Martii commented Oct 12, 2016 •

edited

Loading

Martii commented Oct 12, 2016 •

edited

Loading

nirsharma commented Oct 12, 2016 •

edited

Loading

Martii commented Oct 12, 2016 •

edited

Loading

melroy89 commented Nov 3, 2016 •

edited

Loading

martinlevesque commented Jun 8, 2017 •

edited

Loading

breezewish commented Sep 1, 2017 •

edited

Loading