olback / es6-css-minify

Easily minify CSS and JavaScript in VS Code
https://marketplace.visualstudio.com/items?itemName=olback.es6-css-minify
MIT License
47 stars 13 forks source link

CSS Minify just adds a link to the original file #87

Open steebn opened 5 years ago

steebn commented 5 years ago

Describe the bug In Version 3.1.1: When I minify a CSS file, the minified file is just @import url(<//<filepath>/<filename>.css) to the original file. The output window displays the following:

[<filename>.css]: OK - 0% smaller
   [Warnings]: 1
      - Skipping remote @import of "//<filepath>/<filename>.css" as no protocol given.

To Reproduce Steps to reproduce the behavior:

  1. Go to a CSS file
  2. Click on Minify
  3. Scroll down to Output Window
  4. See error
  5. Open Minified file and see that there is just an @import to the original file.

Expected behavior A truly minified file. I rolled the extension back to version 3.0.3 and the minification works as expected.

Desktop (please complete the following information):

olback commented 5 years ago

Could you provide example code?

steebn commented 5 years ago

Upon further inspection, if the CSS file is saved locally on my machine, it works as expected, there is no issue.

However, if the files reside on a remote machine on my network, then that is when I get the @import url() in the minified file.

I attached a bootstrap.css file that I have on a virtual pc, and the minified version that was created using your extension when I opened the css file in VSCode on my main PC.

This also happens when the files reside on a different server on our network.

bootstrap.css.zip

olback commented 5 years ago

To import CSS you need to specify a URL, that includes a protocol. http:/https:.

To import files from a filesystem, you could do something like this:

@import 'path/to/file';

To import from a url:

@import url(<url-to-file>);
steebn commented 5 years ago

The problem is that I'm not wanting to import. I want the extension to minify the css file that I am working on. However, if I'm using VSCode on my machine, and I open a CSS file that is saved on a remote server/pc on my network, when I minify the CSS file the extension does not minify the file. The .min.css file that is created by the extension is not minified. Rather the extension only writes an import to the existing .css. This behavior does not happen when I roll back the extension to a previous version.

olback commented 5 years ago

Have you changed any settings related to css? What version did you roll back to?

I have a hard time debugging this since I don't have any network drives set up and I use Linux which does not mount network drives in the same way.

steebn commented 5 years ago

So, after my discovery that it is only happening if a file is on a different machine on my network, here are updated steps to reproduce:

To Reproduce Steps to reproduce the behavior:

  1. Open windows explorer and navigate to a different machine on your network. a. Example: \\10.1.24.33\d$ b. Example: \\myServerName\c$ image
  2. Copy or create a normal CSS file to the network location (make sure that there is at at least some css code saved in the file). image
  3. Right click on the CSS file that you saved to the network location, and select Open with Code image
  4. Once the file is loaded in VSCode and there is valid CSS with no errors, click on minify (you will see in the output window that there is a warning): image
  5. Go back to your Explorer window, find the created fileName.min.css, right click and select Open with Code image
  6. You will see that in the minified file, there is just an @import to the original file. image
  7. Then go to your extension, click on the cog, and select Install Another Version image
  8. Choose any previous version (I chose 3.0.1 so that the minify button was still at the bottom) image
  9. Reload VSCode: image
  10. Minify the css file again and presto... it's minified and not just an import (an no output warning) image
steebn commented 5 years ago

Answers below:

Have you changed any settings related to css?

The only setting that I have changed is: "es6-css-minify.minifyOnSave": "exists",

What happens if you import the file directly? (not using url())

Not sure what this means.

What version did you roll back to?

I tried both v3.0.3 and v3.0.1 and they both work correctly

olback commented 5 years ago

Thank you very much.

What do you mean?

My mistake. Not meant to be included :man_shrugging:

I think I know what's going on now. Before 3.1.0 I just passed the content of the css file. CleanCSS (the node module that actually minifies the CSS) had no idea what file it belonged to. That meant that @import did not work. In 3.1.0 this was changed.

I have a silly workaround that should work in the meantime:
Mount 10.1.24.33 to a drive letter and open it with code from that. I have not tested this but if I remember correctly the path will look the same as a local file. Just on another drive.

olback commented 5 years ago

I will try to take a close look when I get home for the weekend.

steebn commented 5 years ago

I have a silly workaround that should work in the meantime: Mount 10.1.24.33 to a drive letter and open it with code from that. I have not tested this but if I remember correctly the path will look the same as a local file. Just on another drive.

Excellent. That did the trick. THANK YOU!! image

I appreciate your help.

JamoCA commented 4 years ago

We use UNC paths to access web applications that are hosted on various network servers. We do not want to have to map each server to a drive letter. (This will overload available drive letters and modify the auditing that we have in place which monitors file save paths.)

I'm currently using 3.2.2 and the JS minification and source map generation appears to work correctly, but CSS minification is useless as all it does is generate an invalid include. Our hostnames are not IP addresses. We use internal hostnames like \\server1 and \\server2.

I know that the CSS minification is treated a differently than JS, but HookyQR VSCodeMinify works correctly with CSS despite the file being saved to on a Windows UNC path.

jhack-jos commented 3 years ago

Same problem here with version 3.3.2, but I think I may have understood the issue. As of now es6-css-minify sends to cleancss the file path like below if the file is in a samba share:

//<filepath>/<filename>.css

but it should instead check if the file is a samba folder (meaning the file is local and not remote), and add the "file:///" protocol prefix to it in such a case before to forward the request to cleancss. This can be done by checking if the first two characters of a path are "//", and than appending the string "file:///" to it.

file://///<filepath>/<filename>.css

If I am not mistaken, Nodejs URL module actually provides a function to do that, called url.pathToFileURL(path). Link to the docs: https://nodejs.org/api/url.html#url_url_pathtofileurl_path.

This behaviour happens because cleancss believes that the resource is remote, but no protocol to fetch it is specified. We can find a hint of this on cleancss page, at https://www.npmjs.com/package/clean-css-cli/v/4.3.0, where on the changelog for version 4.0 is specified:

remote resources without a protocol, e.g. //fonts.googleapis.com/css?family=Domine:700, are not inlined anymore;

olback, do you think you may be willing to implement such a bugfix? I'd be glad to help testing, as I am developing an app that is normally stored on our main samba server.

I think the fix could be implemented nearby https://github.com/olback/es6-css-minify/blob/9f2fe2433e507acc5ff858424769a540349216c1/src/css.ts#L47, since at line 47 of the css.ts source file you forward the file path to cleancss:

const output = new cleancss(this.options as cleancss.OptionsOutput)
// .minify(css);
.minify(typeof _input === 'string' ? css : { [_input.file]: { styles: css } }) // TODO: Use callback

Hopefully, it should be enough to import the URL module and do something like:

let minifyParameters;
if (typeof _input === 'string')
{
   minifyParameters = css;
} else 
{
   const fileURL = pathToFileURL(_input.file);
   minifyParameters = { [fileURL]: { styles: css } }
}

const output = new cleancss(this.options as cleancss.OptionsOutput)
// .minify(css);
.minify( minifyParameters ) // TODO: Use callback
JamoCA commented 2 years ago

Hey @jhack-jos Do your pathToFileURL modifications (from Jan 10) work when accessing UNC paths?

JamoCA commented 2 years ago

Has there been any progress on this bug?

olback commented 2 years ago

Has there been any progress on this bug?

No. See #140.