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

Incorrect sourcemap for multi-line comment when css source from other module #593

Closed
Splurov opened this issue Jun 4, 2015 · 17 comments
Closed
Labels

Comments

@Splurov
Copy link

Splurov commented Jun 4, 2015

https://github.com/Splurov/lpcc-error

How to reproduce:

git clone git@github.com:Splurov/lpcc-error.git
cd lpcc-error && npm install && gulp css

Output:

lpcc-error $ gulp css
[14:54:23] Using gulpfile ~/Work/lpcc-error/gulpfile.js
[14:54:23] Starting 'css'...
Potentially unhandled rejection [2] Error: ENOENT, open '$stdin'

Unhandled rejection goes from https://github.com/jenius/accord/blob/master/lib/sourcemaps.coffee#L17

'$stdin' goes from https://github.com/jakubpawlowicz/clean-css/blob/master/lib/stringifier/source-maps.js#L39

Here is console.log(element) if put it on line 13 in https://github.com/jakubpawlowicz/clean-css/blob/master/lib/stringifier/source-maps.js#L13

[ 'body',
  [ [ 1, 0, '/Users/splurov/Work/lpcc-error/example.less' ] ] ]
{
[ 'color',
  false,
  false,
  [ [ 2, 4, '/Users/splurov/Work/lpcc-error/example.less' ] ] ]
:
[ '#000',
  [ [ 2, 4, '/Users/splurov/Work/lpcc-error/example.less' ] ] ]
;
__ESCAPED_COMMENT_CLEAN_CSS0(1,17)__
[ 'background', false, false, [ [ 4, 2, undefined ] ] ]
:
[ '#fff',
  [ [ 5, 4, '/Users/splurov/Work/lpcc-error/example.less' ] ] ]
}

Looks like a problem in generating sourcemap data for multi-line comment.

clean-css version is 3.3.1

Unfortunately I can't provide clean-css only example, I tried to minimize dependencies, but it works as expected in basic cases.

@jakubpawlowicz
Copy link
Collaborator

Thanks for creating a repo for the error report, much appreciated! I'll take a look at what less does with multiine comments and how we fail to handle it.

jakubpawlowicz added a commit to jakubpawlowicz/lpcc-error that referenced this issue Jul 1, 2015
@jakubpawlowicz
Copy link
Collaborator

I did some investigation and it does not seem to be a clean-css issue. I managed to solve your problem by passing sourceMapInlineSources: true to less.js clean-css plugin, see https://github.com/Splurov/lpcc-error/pull/1/files

If you take a look at https://github.com/jakubpawlowicz/clean-css/blob/master/lib/stringifier/source-maps.js#L14 you'll see it can't be a problem with multiline comments since clean-css ignores string tokens - see line 19.

I have also found that issue which seems similar - less/less-plugin-clean-css#8

@lukeapage
Copy link
Contributor

The problem is that something is adding $stdin as a source.

accord is then trying to read it...

stdin

which errors.

clean-css shouldn't be adding a $stdin file as a source, so either clean-css or something it relies on or the way it is called is at fault. I'll continue investigating.

@lukeapage
Copy link
Contributor

and it seems that when clean-css is adding the tokens, the background token, just after the multi-line comment is missing the source property.

missing-source

Now the question is.. is that less malforming a sourcemap input or clean-css ?!

@jakubpawlowicz
Copy link
Collaborator

Thanks @lukeapage! The interesting bit is that if I run it from CLI there's no reference to $stdin but to both CSS and Less files.

I'll give it a try later on.

@lukeapage
Copy link
Contributor

I think this is the problem

var lineBreak = require('os').EOL;

less uses just \n I think - or anyway the source you are getting only has \n in it...

No-one has just \r so I would have thought you can just use \n ?

@lukeapage
Copy link
Contributor

Will leave it to you.. changing the line break didn't fix the problem, though it did change things that looked important 😕

@lukeapage
Copy link
Contributor

@jakubpawlowicz do you want to re-open this so its not forgotten? or did you get somewhere?

@jakubpawlowicz
Copy link
Collaborator

Yeah, let's reopen it.

@jsguy
Copy link

jsguy commented Sep 2, 2015

Hi @jakubpawlowicz,

Any updates on this?

@jakubpawlowicz
Copy link
Collaborator

I have had no time to work on this yet but I may have in the coming weeks.

@EightArmCode
Copy link

+1

I'm using node-sass piped to autoprefixer to cleancss. The source map file appears as $stdin.

@rros
Copy link

rros commented Dec 12, 2015

Just found out this issue is probably the source of my issue. See linked issues in gulp-less and less.js.

It seems clean-css assumes the line breaks of input sourcemaps are in format of the current platform it's running on: var lineBreak = require('os').EOL.

Changing the lineBreak to var lineBreak = "\n"; seems to help in some cases, but I think this isn't the solution. As far as I know (or could find), you can't assume anything about line breaks in sourcesContent.

The code which tries to match line breaks can be found in comments-processor.js: https://github.com/jakubpawlowicz/clean-css/blob/v3.4.8/lib/text/comments-processor.js#L72-L95

The other text processor files contain similar line break matching code. Although I haven't had issues with them, I assume this can cause this issue as well. Related files:

My guess is these processors have to detect the line breaks used in each of the elements of sourcesContent instead of assuming the OS line breaks.

@jakubpawlowicz Is there any way I can help you resolve this issue?

@jakubpawlowicz
Copy link
Collaborator

Thanks @rros for detailed report. You are right about line breaks, those are OS line breaks by default. We can definitely detect line breaks (you can't mix different line breaks in a single file, can you?) but as a scope of a next minor clean-css version.

Btw, if you use git you can force line breaks on a repository level. Have no idea if that helps you as a workaround: https://help.github.com/articles/dealing-with-line-endings/

@rros
Copy link

rros commented Dec 17, 2015

@jakubpawlowicz thanks for the suggestion. We use Mercurial for version control and just changed the line breaks of the less files for now. However we primarily use Windows machines, so when we add a new file, it will have windows line breaks by default. As a workaround this is fine, but I like to get this issue fixed :)

Regarding mixing line breaks, I think there are two cases here:

  1. A input file has mixed line breaks
  2. Multiple input files have different line breaks

I think the first case is just bad input and we can ignore it, but the second is probably common. For example when I import a less-file (line break \n) from a bower component where my own input file has \r\n like breaks. This results in sourcesContent having some entries with \n line breaks while others have \r\n line breaks.

@jakubpawlowicz
Copy link
Collaborator

Thanks @rros for a valuable suggestion regarding mixed line breaks! I agree, we should handle it gracefully as people may be coming from different environments.

@jakubpawlowicz jakubpawlowicz modified the milestones: 4.0, ~future~ Jan 4, 2017
@jakubpawlowicz
Copy link
Collaborator

clean-css 4 got rid of content escaping and deals with windows and *nix line breaks interchangeably. Internally all line breaks are removed and upon serializing written as \r\n on Windows and \n on Unix.

Please reopen this issue (or open a new one) if this continues to be a problem.

@jakubpawlowicz jakubpawlowicz removed this from the ~future~ milestone Jan 25, 2017
dalf added a commit to dalf/searx that referenced this issue Mar 21, 2021
Close searx#2670

Note: clean-css contains a bug:
* a multiline comment or URL adds "$stdin" to the sourcemap (see src/less/logicodev/search.less)
* in this case when the user opens the devtools, the browser fails to load this "https://.../$stdin" URL
* it is not a problem and the error appears only when the user actively tries to debug the CSS.
* seems related to clean-css/clean-css#593
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