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

Jump to bottom

support CSS sourcemaps #16985

Open
Findus23 opened this issue Dec 20, 2020 · 1 comment
Open

support CSS sourcemaps #16985

Findus23 opened this issue Dec 20, 2020 · 1 comment
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. Technical debt Issues the will help to reduce technical debt
Milestone
Backlog (Help wan...

Comments

@Findus23
Copy link
Member

This is one thing that in my opinion would make developing CSS in Matomo a lot easier and more fun.
And now that we are using wikimedia/less.php it is finally possible to achieve:

Issues like #16983 are unnecessarily hard to debug as all you see in the browser developer tools is index.php:774 which contains a huge amount of CSS.

Instead (probably only when development mode is enabled) Matomo should add an endpoint like /index.php?module=Proxy&action=getCssSourcemap that returns the CSS sourcemap, add a link to it at the end of the CSS output and therefore allow the browser to show helpful things like somefile.less:123 allowing to immediatly fix the right file.

Reasons why this might not be completely trivial (but still probably not that complex):

  • the LESS compilation code would need to be rewritten in the less.php class style instead of the lessc compatibility style (but that should just be a few lines of code)
  • I don't see a method that just returns the content of the source map instead of writing it into a file
  • A new asset handling and Proxy endpoint would need to be added

If this is too much work, there would be an easier method that only needs a rewrite of the LESS compilation code:

  • set $options = array( 'sourceMap' => true ); and therefore append the sourcemap to the CSS file
  • this means bloating the file size of the CSS and therefore the page load time (during the development) a lot
  • but nothing else needs to be changed.
@Findus23
Copy link
Member Author

Implementing the latter is really easy

Patch 
diff --git a/core/AssetManager/UIAssetMerger/StylesheetUIAssetMerger.php b/core/AssetManager/UIAssetMerger/StylesheetUIAssetMerger.php
index c57610a466..c6652e4cf6 100644
--- a/core/AssetManager/UIAssetMerger/StylesheetUIAssetMerger.php
+++ b/core/AssetManager/UIAssetMerger/StylesheetUIAssetMerger.php
@@ -9,7 +9,7 @@
 namespace Piwik\AssetManager\UIAssetMerger;
 
 use Exception;
-use lessc;
+use Less_Parser;
 use Piwik\AssetManager\UIAsset;
 use Piwik\AssetManager\UIAssetMerger;
 use Piwik\Common;
@@ -21,7 +21,7 @@ use Piwik\Plugin\Manager;
 class StylesheetUIAssetMerger extends UIAssetMerger
 {
     /**
-     * @var lessc
+     * @var Less_Parser
      */
     private $lessCompiler;
 
@@ -40,12 +40,13 @@ class StylesheetUIAssetMerger extends UIAssetMerger
     protected function getMergedAssets()
     {
         // note: we're using setImportDir on purpose (not addImportDir)
-        $this->lessCompiler->setImportDir(PIWIK_DOCUMENT_ROOT);
+        $this->lessCompiler->SetImportDirs([PIWIK_DOCUMENT_ROOT]);
         $concatenatedAssets = $this->getConcatenatedAssets();
 
-        $this->lessCompiler->setFormatter('classic');
+//        $this->lessCompiler->SetFormatter('classic');
         try {
-            $compiled = $this->lessCompiler->compile($concatenatedAssets);
+            $this->lessCompiler->parse($concatenatedAssets);
+            $compiled = $this->lessCompiler->getCss();
         } catch(\Exception $e) {
             // save the concated less files so we can debug the issue
             $this->saveConcatenatedAssets($concatenatedAssets);
@@ -62,6 +63,7 @@ class StylesheetUIAssetMerger extends UIAssetMerger
 
         $this->mergedContent = $compiled;
         $this->cssAssetsToReplace = array();
+//        var_dump($compiled);
 
         return $compiled;
     }
@@ -108,7 +110,7 @@ class StylesheetUIAssetMerger extends UIAssetMerger
     }
     
     /**
-     * @return lessc
+     * @return Less_Parser
      * @throws Exception
      */
     private static function getLessCompiler()
@@ -116,7 +118,8 @@ class StylesheetUIAssetMerger extends UIAssetMerger
         if (!class_exists("lessc")) {
             throw new Exception("Less was added to composer during 2.0. ==> Execute this command to update composer packages: \$ php composer.phar install");
         }
-        $less = new lessc();
+        $options = array( 'sourceMap' => true );
+        $less = new Less_Parser($options);
         return $less;
     }

Unfortunately there are other things that make this much harder as expected: Instead of (as I expected) generating a list of LESS imports for all files and compiling this getConcatenatedAssets() already appends all files into one huge LESS file meaning any information about files is already lost before the LESS compiler.

@tsteur tsteur added this to the 4.3.0 milestone Dec 20, 2020
@tsteur tsteur added the c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. label Dec 20, 2020
@mattab mattab added the Technical debt Issues the will help to reduce technical debt label Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. Technical debt Issues the will help to reduce technical debt
Projects
None yet
Milestone
Backlog (Help wanted)
Development

No branches or pull requests
3 participants
@tsteur @mattab @Findus23