Recompile less when changing imports

[madebysoren]

It would be really cool if less documents was recompiled whenever changes are made in their "child" documents. So if I import another less document and make changes to this, the "parent" document should be recompiled as well, so I don't need to open that one and make a change to save it again and force a recompilation.

Thanks for doing an awesome good job on this!

[blowsie]

Id love to see this feature, would be a massive time saver.

[ctolkien]

This is really important for us as well. We've had to use other methods to work with LESS as changes made to our LESS file (included at the end of Bootstrap.less) are not reflected unless you also save Bootstrap.less.

[SLaks]

Implementing this would require that WE scan your entire project for LESS files every time you save, to find and update referencing files.
This would be a perf hit.

I'd like to write a file dependency cache which will cache the reference information and refresh itself on save and on project reload to avoid this perf hit. (this would also help perf for bundle updates)

I'll probably write that soon.

[madskristensen]

Such a cache exist and is public. I forgot the name...

But it doesn't walk up the chain

[SLaks]

In what assembly?

[madskristensen]

Probably in Web.Extensions

[SLaks]

So far, I found Microsoft.CSS.Editor.DocumentImportCache (internal)

[madskristensen]

Yes, that's it

[madskristensen]

No wait, it's specific to LESS so it's probably not in the CSS assembly

[madskristensen]

But still, it only walks down the import chain. Not up

[SLaks]

Microsoft.Less.Core.LessReferenceCache, also internal 😦

[madskristensen]

The interface is probably public. Then do an [Import] if I remember correctly

[SLaks]

There is no interface

[SLaks]

I had in mind a wrapper around ConcurrentDictionary<string, ISet<string>> that takes a Func<string, IEnumerable<string>> to get the dependencies of a file.
It would clear / repopulate the cache in VS and/or FSW event handlers

[madskristensen]

I'll ask the developer when I get back to the office Monday then

[madebysoren]

Great to see that you are considering how to implement this. I should add that it would be awesome to have the less variables from the parent document available in the child documents as well. I know it works when compiling, but I am still getting a less error saying that the variable is undefined.

Thank you for the good work!

[SLaks]

@madebysoren That should work already.
What exact scenario is failing?

Note that I'll be rather busy for the next few weeks, so I probably won't implement this before December.

[madebysoren]

Maybe I am just using it the wrong way. I'm still pretty new to this. I have the following structure:

general.less:

@red: #f5406b;
@import "another";

another.less

.someclass {
    background: @red;
}

This would give me the following error: "LESS: variable @red is undefined". And btw December is perfect. No stress. I'm just delighted that you guys are actually making this stuff.

[am11]

@madebysoren, apparently you need to (re)define @red variable in another.less or import general in another (instead the other way around)..

[SLaks]

You're using it backwards from the way variables are usually used:

variables.less:

@red: #f5406b;

mypage.less:

@import "variables";
.someclass {
    background: @red;
}

Supporting that approach would require me to hook my dependency logic into Microsoft.Less.Editor.Intellisense.VariableCompletion, which is internal and non-extensible.

[madebysoren]

Of course. That makes much more sense. Thank you.

[SLaks]

@madskristensen, can you make this logic extensible for the next VS update?

This problem is especially annoying in Bootstrap, which has about a dozen LESS files that are imported by bootstrap.less, starting with variables.less and mixins.less.
All of these files use variables & mixins from those first two imported files, but Visual Studio doesn't know that, giving lots of errors.

[SLaks]

@am11 He doesn't need to; his approach is perfectly fine.
That is only needed to make VS' LESS services work.

[am11]

@SLaks, indeed. I didn't thought about the variable scope and that it doesn't take all the variables in the scope into consideration. Only "walking down" as Mads put it.

@madskristensen, any word on the existing scope-aware cache?

[madskristensen]

Since any .less file can be referenced by multiple parent .less files, walking up the import chain isn't really a good idea. Just because bootstrap has chosen this approach doesn't mean that everyone else should

[sitefinitysteve]

I'd love to see this taken care of, even with a perf-hit...a php\sublime guy here who's using VS2013 keeps WinLess open in the background and shuts off WebEssentials since it does do the recompile on _import save.

...I'm considering the same thing because it's so inconvenient

[bdukes]

Walking up the import chain is something that CodeKit implements, so it's been painful for our Mac guy to use VS, since it doesn't provide that. If your tools don't work with the most popular project using the framework, I'd suggest it may be worth looking into getting that working, even if it's difficult to do performantly. When we were investigating less and trying to decide how to structure our code, we looked to bootstrap as an example, and I'd guess we're not alone in that. I totally understand that what I'm asking may be a very large undertaking, but I hope my feedback is useful.

As an alternative, when we used Sublime, the less compiler we used could be setup to compile a specific file every time any less file in the project was saved, which helped.

I'll have to see if WinLess will work for us, instead of Web Essentials, for the time being, thanks @stevescotthome :-)

[ctolkien]

We've had our front end guys using node and less-watch package, however there are differences between the way it compiles paths for images and Web Essentials.

Basically, this remains a huge pain point, a simple setting with regard to recompiling a specific file on save (so we can point it at bootstrap) is, well, shortsighted but it would solve all the issues we have.

[madskristensen]

here's the API:

IDocumentImportManager manager = Microsoft.CSS.Editor.CssDocumentHelpers.DocumentImportManager;
IEnumerable<IDocumentReference> imports = manager.GetImports(either ITextBuffer or StyleSheet);
manager.ImportsChanged += OnImportsChanged;

[am11]

@madskristensen, that's great!

Which means; we can maintain a simple tree in LessMargin and batch compile for each node. Since we don't have batch compilation with node compiler (at least from the CLI), we can do something like this:

// using Microsoft.CSS.Editor.CssDocumentHelpers;
IDocumentImportManager manager = DocumentImportManager;
IEnumerable<IDocumentReference> imports = manager.GetImports(fileSaved);
foreach(var document in imports)
{
    foreach(var lessFilePath in _importTree.FindParents(document.SourceUri.AbsolutePath))
        LessCompiler.Compile(lessFilePath);
    ...
}

Also, we can look into the way to batch-compile within node environment. Then write a JS file on-the-fly and execute node [params] on-the-fly.js. This will save a major perf-hit.

[SLaks]

BTW, I just finished rewriting the margin code; it is now far far smaller.
SLaks@a467355
https://github.com/SLaks/WebEssentials2013/tree/confoxide/EditorExtensions/Margin

This code would go in a derived version of https://github.com/SLaks/WebEssentials2013/blob/confoxide/EditorExtensions/Compilers/CompilerRunner.cs#L104 to affect all compilations to disk.

[SLaks]

For batch-compile, see #381; this would only touch the NodeExecutorBase layer.

[madskristensen]

@SLaks are you ready to send the PR? Perhaps we should wait till the weekend to release 1.7 so we get more time to test all the new code

[SLaks]

No; it doesn't even compile yet.

[madskristensen]

Ok

[SLaks]

I still need to finish writing the compiler providers (very simple), rewrite minification as a compilation listener & a save listener consuming MEF-ContentType-exported minifiers, and move the compile-on-build logic to one place.

Then comes unit tests for everything.

[am11]

@madskristensen, @SLaks, the AncestryManager.cs is a almost ready. I can't figure out how to implement ProjectHelpers.GetAllSourceDocumentReferences() method to return IEnumerable<IDocumentReference> for all LESS or SASS files in the project.

Please take a look whenever its convenient.

[tidyui]

Wouldn't it be sufficient to be able to specify how dependencies are set up, what should trigger a recompile like for example CodeKit does. This feature would include:

• Some kind of GUI in the property pages for the Web Essenstials
• Saving a config-file in the project root when saving the above config
• Parsing the config on project load & file change
• Setting up FileWatchers for the files marked as dependencies and performing appropriate actions.

This way it's up to the developer to set up dependencies the way he/she wants. For example:

assets\less\general.less (do not compile)
assets\less\typography.less (do not compile)
assets\less\screen.less (compile on change, dependent on general.less & typography.less)

Seems like a simple way of implementing it, plus there's always a big risk of making bad assumptions when walking some kind of dependency chain and deciding the what behavior the develop wants.

Storing the config in the project root also makes it easy to check in the configs with the source code making sure that all developers on different machines have the same config for project minification.

[am11]

@tidyui, we can certainly maintain a blacklist in settings.

Here is the latest version of ancestry maker:

// In ProjectHelpers

// Call this on solution load and save it as var ancestryDictionary.
public static Dictionary<string, HashSet<string>> GetAncestory()
{
    var ancestry = new Dictionary<string, HashSet<string>>();
    var files = new[] { "*.less", "*.sass" }.SelectMany(extension =>
                Directory.EnumerateFiles(GetSolutionFolderPath(), 
                                         extension,
                                         SearchOption.AllDirectories));

    foreach (var file in files)
    {
        ancestry = UpdateAncestryForSingleDocument(file, ancestry);
    }

    return ancestry;
}

// We can use it as the event handler onSavng Sass or Less document.
public static Dictionary<string, HashSet<string>> UpdateAncestryForSingleDocument(
                                                 string filePath,
                                                 Dictionary<string, HashSet<string>> ancestry)
{
    var directoryName = Path.GetDirectoryName(filePath);
    var paths = new CssItemAggregator<string>(s => s is ImportDirective)
                    .Crawl(new CssParser().Parse(File.ReadAllText(filePath), false))
                    .SelectMany(s => s.Split(' ', ','))
                    .Select(s =>
                    {
                      var path = Path.Combine(directoryName, s + Path.GetExtension(filePath));
                      return Path.GetFullPath(path);
                    });
    // Now paths is IEnumerable<string>.

    foreach (var path in paths)
    {
        var item = ancestry[path] ?? new HashSet<string>();

        item.Add(filePath);
        ancestry.Add(path, item);
    }

    return ancestry;
}
// Now compile all files returned by ancestryDictionary[targetLessOrSassFilePath]
// TODO: cache results? Depends on how expensive is CssItemAggregator.Crawl()

Before calling ancestryDictionary[targetLessOrSassFilePath] we can tally with blacklist; if this file is marked as Don't Comple.

[am11]

@SLaks, is @import url(path/to/another.less) invalid in LESS? When I use that syntax, it throws NullReferenceException at selector(typedItem) in DFS. I tried putting null check there but still getting exception. Also, new CssItemAggregator<string> { (ImportDirective i) => i == null ? "" : i.FileName.Text } had no effect.

BTW, adding a new setting is SUPER easy now! 👍

[am11]

@madskristensen, this should be closed with #589.