Less file not listed in sources sourcemap array if it contains only variables

[davej]

If I import a less file that only includes variables then it does not get included in sources. Here's an example, notice that the map file at the end does not include variables.less as a source.

main.less

@import "variables.less";

a { color: @theme-primary; }

variables.less

@theme-primary: #2C3E50;

main.css.map

{
  "version":3,
  "sources":["/less/main.less"],
  "names":[],
  "mappings":"AAGA;EACC,…"
}

[seven-phases-max]

And how it's supposed to get there? The sourcemap only maps generated CSS code (selectors, properties etc.) to its corresponding Less code, and variables on there own never produce any CSS code at all. So no Less variable -> CSS code mapping in the output sourcemap and thus no "variable only" Less file is referenced.

Unless you're also expecting just #2C3E50 value of the generated color: #2C3E50 to be mapped to the @theme-primary: #2C3E50; definition, but no, this is not how it works. A minimal entity for the source map is a complete property: value(s); statement and not individual values there. Thus even if Less could generate such, a browser barely could get it properly... See for example how they treat pretty much related "individual elements of a selector" stuff: #1492).

[davej]

In other words: the "source" array is not "list of files used to generate this", but just "list of names/paths of the files referenced in the "mappings".

Do you have a source (no pun intended) for that? Everything I've seen in the spec and elsewhere says something along the lines of "An array of URLs to the original source files". I guess that's somewhat ambiguous, but to me 'variables.less' is a source file even if it doesn't directly map 1:1 with a line of code.

Other compilers like Sass and Stylus also follow my interpretation of what should be in the sources array.

For example with Sass:

main.scss

@import "_variables.scss";

a { color: $theme-primary; }

_variables.scss

$theme-primary: #2C3E50;

main.css.map (excerpt)

{ "sources":["/styles/main.scss","/styles/_variables.scss"] }

---

I will explain why this is important to me because otherwise it might seem like I'm nitpicking...

I'm creating a compile and live-reload tool (https://github.com/davej/piggy-in-the-middle/). It compiles less/sass/stylus/coffeescript/babel etc.. and re-compiles whenever there is a change in a source file which is read from the sources array in the sourcemap. Therefore the tool doesn't work if a source file isn't included in the sources array. As far as I can tell this is not a problem with other compilers.

I'm happy to jump into the code and create a PR for this if it would be accepted 😀

[seven-phases-max]

Do you have a source (no pun intended) for that?

Source Map Revision 3 Proposal, page 4.

I guess that's somewhat ambiguous, but to me 'variables.less' is a source file even if it doesn't directly map 1:1 with a line of code.

It makes sense... But for your goal in general I'd say it's like you're trying to interpreter rather simple "mapping" table (after all this is why it's named "SourceMap") like a full-featured and comprehensive debugging-info/reflection/reverse-engineering database. (Nothing wrong, just a bit weird if the whole idea is based on a not initially intended facility).

Other compilers like Sass and Stylus also follow my interpretation of what should be in the sources array.

Good for them, but others then are free to complain of rather redudant and bloating data most of users cannot ever see/use (so if it's to be interpreted like it's time for an Independend SourceMap Spec. (Ext.) fork :).

P.S. Though technically, if the PR breaks nothing I see nothing wrong with it too (so changing labels).

[davej]

Thanks for that link, I read it before but missed the part about it being "used by the mappings".

I wonder could this be considered anyway. I'll give a few reasons:

1. Practically speaking, the other compilers seem to have implemented it in the way I have described. It would be nice to be consistent.
2. Being able to parse all the sources is useful for a number of reasons, including the reason I want it included for my tool (see comment above).
3. Although this isn't included in the spec, It doesn't break with the spec, it's just superfluous.
4. It is useful for browser tooling. For example, with Chrome the file gets included in the sources panel which is super useful (see sass screenshot below). Without it, then the sources are incomplete because you can't figure out where/what the variables are defined as.

[seven-phases-max]

The only problem is that as soon as we start using the sourcemap as a sort-of automatically generated "project settings" file (or at least "watch-list" file), issues are starting to pile up quite quickly: for example it's not just Less (or whatever preprocessor) files may be used in the build but also others, i.e.:

• external JS plugins (Less and PostCSS)
• external fonts/images/icons (hello data-uri)
• etc.

According to the new approach all of these now should also be referenced in the sourcemap (I'm not even counting that some of that external stuff may also reference even more external stuff and so on and so on).

[davej]

Ok, am I better off waiting for more input on this before doing a PR? i.e. Do you think there is a good chance the PR will be rejected?

By the way, my proposal would be to only include Less files. I think the Chrome screenshot I've referenced above makes a compelling case for the feature by itself.

[seven-phases-max]

Ok, am I better off waiting for more input on this before doing a PR?

I'd wait for @matthew-dean opinion.

[matthew-dean]

Hmm. My understanding up to this point of using source maps was that files that don't have a direct mapping aren't listed in sources, but.... some of that understanding (or mis-understanding) may have come from trying to understand how Less generates source maps.

For the most part, I think Less maps are ruleset-based. I do know that the way Less is parsed doesn't actually allow as much resolution on maps as the spec defines (because Less has always only marked the start character of a parsed object but not the end character). And part of that not seeming like a problem in the past is that developer tools typically just show ruleset mappings.

As @seven-phases-max mentioned, there are sources pulled from that don't really make sense (or do they?) in the list of sources, such as fonts. There would be no mappings from point A in a .less file to point B in a font file.

On the other hand..... if a Less JS plugin rendered a Less node, being able to trace the source back to that function would be pretty amazing.

@davej When I was creating my compile-as-you-go app, I originally thought about using sources as well, and probably realized I couldn't when Less returned its results. I ended up having to execute compiling in a separate spawned Node thread, and then I monkey-patched the Node FS module to assemble a list of every file that was being requested.

So, long and short, if you're trying that approach, and finding the Less.js is the only one that doesn't list all sources retrieved while compiling, then it should definitely be changed. The specification doesn't spell it out entirely what should happen, especially if a section of code, in reality, has a "chain" of sources to render that one piece.

On the flip side...... some decisions would need to be made. If you include the original source in sourcesContent, then binary sources would, I think, need to be excluded. However, you could still keep them in the sources key by setting the corresponding sourcesContent to null for things like fonts or images that are rendered inline. Just mentioning that because if this is added, there will need to be tests added to make sure that binary content doesn't end up in the sourcesContent when that option is set (and that it doesn't just omit that value in the sourcesContent array or mappings values leading to mis-matched mappings).

So.... final opinion from me - considering the inclusion of sourcesContent is a value in the spec in the first place, it's clear that source maps are designed to be as verbose as necessary. I see no inherent harm in including every referenced source, no matter what that source might be. It's possible this would have saved me loads of time approaching this problem a few years ago similar to you're facing now.

Weighing all that, I'd vote yes.

[matthew-dean]

Ideally, this should be addressed along with #2974.

[davej]

Thanks!

Ok, I'll try to schedule this in over the next couple of weeks. I'm completely new to the less codebase so any pointers on where to get started when working on this, or potential gotchas.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[rjgotten]

@matthew-dean

So, long and short, if you're trying that approach, and finding the Less.js is the only one that doesn't list all sources retrieved while compiling, then it should definitely be changed. The specification doesn't spell it out entirely what should happen, especially if a section of code, in reality, has a "chain" of sources to render that one piece.

FYI; yes. Less is the odd one out.
It should list all original (.less, atleast) source files that contributed. If you don't, you also end up with partially broken source maps when the maps are processed further by other tooling such as webpack.

Webpack will fill in any missing files with their compiled result when it - rightfully - assumes that if there's nothing to chain off of, you're actually dealing with a true source file.

E.g. if your less entrypoint only contains @import instructions, then Webpack will substitute the fully compiled result as the 'source' and will list that in your source map.

I would actually classify this as a BUG as it profoundly messes up downstream debugging tools. Which is precisely what source maps are meant for, in the first place.

[rjgotten]

@matthew-dean
I've done some experimenting late last week and I think it's possible to force unmapped files into the sourcemap by adding a dummy mapping.

When picking this up, you might as well fix another issue related to source-maps as well:
The Less compiler currently seems to output filesystem paths for the sources array, atleast under some conditions. (Managed to replicate that through the less-loader package for Webpack, atleast.)

Those are supposed to be URLs and failure to comply with that leads to a lot of malformed sourcemap source paths on the Windows OS, where the filesystem path separator is \, which does not correspond with the URL path separator /.

See e.g. mozilla/source-map#91 or mozilla/source-map#355

(Source map handling seems to be a huge mess across the board when it comes to anything within the Webpack ecosystem, it seems. It'll need fixing from front-to-back, so might as well start at the compiler itself.)