Skip to content
This repository has been archived by the owner on Mar 12, 2023. It is now read-only.

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

Open
steebn opened this issue Sep 11, 2019 · 15 comments
Open

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

steebn opened this issue Sep 11, 2019 · 15 comments
Assignees
Labels
bug Something isn't working verified This issues is confirmed to be an issue.

Comments

@steebn
Copy link

steebn commented Sep 11, 2019

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):

  • OS: Windows_NT x64 10.0.18362
  • VS Code:
    • Version: 1.38.0 (system setup)
    • Commit: 3db7e09f3b61f915d03bbfa58e258d6eee843f35
    • Date: 2019-09-03T21:49:13.459Z
    • Electron: 4.2.10
    • Chrome: 69.0.3497.128
    • Node.js: 10.11.0
    • V8: 6.9.427.31-electron.0
@olback
Copy link
Owner

olback commented Sep 11, 2019

Could you provide example code?

@olback olback added bug Something isn't working critical not verified Issue not verified labels Sep 11, 2019
@olback olback self-assigned this Sep 11, 2019
@steebn
Copy link
Author

steebn commented Sep 11, 2019

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

olback commented Sep 12, 2019

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

steebn commented Sep 12, 2019

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

olback commented Sep 12, 2019

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.

@olback olback removed the critical label Sep 12, 2019
@steebn
Copy link
Author

steebn commented Sep 12, 2019

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

steebn commented Sep 12, 2019

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

olback commented Sep 12, 2019

Thank you very much.

What do you mean?

My mistake. Not meant to be included 🤷‍♂️

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

olback commented Sep 12, 2019

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

@olback olback added verified This issues is confirmed to be an issue. and removed not verified Issue not verified labels Sep 12, 2019
@steebn
Copy link
Author

steebn commented Sep 12, 2019

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

JamoCA commented Mar 7, 2020

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

jhack-jos commented Jan 10, 2021

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

.minify(typeof _input === 'string' ? css : { [_input.file]: { styles: css } }) // TODO: Use callback
, 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
Copy link

JamoCA commented Dec 8, 2021

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

@JamoCA
Copy link

JamoCA commented Aug 5, 2022

Has there been any progress on this bug?

@olback
Copy link
Owner

olback commented Aug 5, 2022

Has there been any progress on this bug?

No. See #140.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working verified This issues is confirmed to be an issue.
Projects
None yet
Development

No branches or pull requests

4 participants