Support sourcemaps with browser side compiler #1541

Open
jadus opened this Issue Sep 11, 2013 · 22 comments

Projects

None yet

7 participants

@WraithKenny

Regardless of how to feed the sourcemap to the browser, I'd be handy for the browser compiler (at least optionally when invoked manually) to be able to return a sourcemap file as a string.

I use LESS (in conjunction with CodeMirror) in a WordPress Plugin, and store/return/write files using WP's ajax handlers. If I could get a sourcemap from the browser's less.js, I could handle writing the sourcemap to the correct location, and append the css comment myself. I just need the sourcemap exposed in the browser api.

I'm also trying to get LESS into WP core: http://core.trac.wordpress.org/ticket/22862 for details.

@lukeapage
Member

Yes, there is an option you will be able to override that is a function
called with the sourcemap output. I will add more details once this works
in the browser version.

@lukeapage
Member

@WraithKenny
adding this option will work in beta1 & 2 I think.

less.writeSourceMap = function(stringSourceMap) {
}

You can set all the normal sourcemap options lessc has, using the right casing as options on the less object.

@lukeapage
Member

hrmm, the problem is https://github.com/mozilla/source-map which we use to generate sourcemaps. a) is the license compatible? (I don't know) b) the build can't be easily included into less.js and I'm not sure we would want to just include it.

I have just made a commit allowing the sourcemap to be a data uri, and that works. I've also made the sourcema generator an option so that someone can push it in from the browser. More work needs to be done to decide how this should work.

@jadus
jadus commented Sep 14, 2013

"SyntaxError: Cannot read property 'SourceMapGenerator' of undefined"
Am I missing something ?

My options are :

            less = {
                env: "development", 
                async: false,
                poll: 2000,
                sourceMap: true
            };

Thanks

@lukeapage
Member

@Jadus I may need to add the option to the env file to get it passed through from the less object to the source map output class.. sorry about that, was in a rush to finish for today

@lukeapage
Member

I have got data-uris working fine, but chrome isn't liking sourcemaps in inline stylesheets. I have a test case and will raise a bug.

@lukeapage
Member

Information added here : https://code.google.com/p/chromium/issues/detail?id=285786

I will publish information on how to make this work on the browser once that issue is resolved

@jadus
jadus commented Oct 15, 2013

Is this working with the last release ?

@lukeapage
Member

The linked issue prevents it working in chrome. Yes the latest release has a fix in, but to get it working you have to be creative.. I haven't worked out how to include the Mozilla source maps js with less or whether the licences allow it. For lessc the package is included with npm.

@ktiedt
ktiedt commented Mar 27, 2014

Just a reference in regards to the potential "legal blocker": http://www.apache.org/legal/3party.html -- If you scroll down you will find a list of Licenses that Apache Foundation projects are allowed to use/include as part of their projects... BSD is one of them, which should make use of the source-map generator that is BSD licensed in an Apache Licensed project safe... -- I am not a lawyer, but that seems like a pretty solid assumption based on the above link.

@englercj
englercj commented May 9, 2014

Hey @lukeapage this is obviously still an issue. Did you ever end up creating a new chromium issue after the one you made was closed? I searched briefly and didn't find it.

This is an annoying issue for me since I got everything working except the Chrome bug not reading the source map.

@lukeapage
Member

I didn't raise another no, I haven't had much luck with chrome bugs - everything I find seems to be not mainstream enough for them to fix.
Unless chrome fixes it, I don't have much drive to get less working, as you said it isn't too much work.
If we did include it in the browser version we would also have to look at it being optional I expect because of size etc.
I'm still a bit confused why people still use less in the browser for development.. especially with server side sourcemaps, you can set up a grunt watcher, edit the less inside chrome and have the output be picked up.

@englercj

I'm still a bit confused why people still use less in the browser for development.. especially with server side sourcemaps, you can set up a grunt watcher, edit the less inside chrome and have the output be picked up.

Still really useful in ecosystems where not all developers on a project are using node/grunt; but they still need to view certain pages. Less in the browser is one of many tools to help not have to run builds in dev and reduces requirements of tools for non-node devs, which is always good.

@ChrisCinelli

Annoying. I would like to see this fixed too. I starred the bug at https://code.google.com/p/chromium/issues/detail?id=285786
I would encourage anybody interested in this to do the same.

@ChrisCinelli

BTW, @lukeapage did you look at: https://github.com/thlorenz/inline-source-map and https://github.com/gromnitsky/coffee-inline-map ?
I wonder if you need to do a base64 conversion to make it work.

@lukeapage
Member

So less has two inline options, one to inline the source files into the
map, another to inline the map into the css using a data uri.

It hadn't occurred to me to have a css link tag with a data uri rather than
a style tag containing the css..

Its possible it might work.. it would have to be a new browser mode since
it definitely won't always work in older browsers (32kb limit).

I don't have time to try it, I have little less time at the moment and am
concentrating on trying to finish 2.0

@jadus
jadus commented Jul 23, 2014

What should I do to try this ?
I still get "SyntaxError: Cannot read property 'SourceMapGenerator' of undefined"

with these options :

less = {
                env: "development", 
                async: false,
                poll: 2000,
                sourceMap: true
            };
@jadus
jadus commented Sep 16, 2014

nothing new here ?

@lukeapage
Member

@jadus no, not really. Chrome doesn't work (see comments above) and so we haven't got a browser version working with sourcemaps - its not our top priority at the moment (our main use-case we support is server side compilation, using something like live reload and a grunt or gulp watcher). If chrome at least supported it or another browser started supporting css sourcemaps (maybe mozilla does now?) AND supported definitions inline, which chrome didn't it might give us some motivation. But we don't really have much of a reason at the moment.
We are happy for you to create a pull request (on the 2.0 branch) - where it should be easier anyway because we use browserify if you like.

@lukeapage
Member

p.s. the reason you get an exception is we just don't define SourceMapGenerator in the browser - we don't include mozillas source code, so you get an exception (that and some configuration is probably all thats missing on our side). Really it should give a nicer error.

@rubyruy
rubyruy commented Mar 18, 2015

For what it's worth I tried hacking in the moz SourceMapGenerator and emitting to a link tag (with a disgustingly large base64 data-href)- and sure enough, it makes source-maps work great using browser compilation in chrome.

Other than plugging in an environment (with getSourceMapGenerator and encodeBase64 - btoa seems to work fine for the latter), the main change was modifying createCSS thusly:

createCSS: function (document, styles, sheet) {
        // Strip the query-string
        var href = sheet.href || '';

        // If there is no title set, use the filename, minus the extension
        var id = 'less:' + (sheet.title || utils.extractId(href));

        // If this has already been inserted into the DOM, we may need to replace it
        var oldStyleNode = document.getElementById(id);
        var keepOldStyleNode = false;

        // Create a new stylesheet node for insertion or (if necessary) replacement
        var styleNode = document.createElement('link');
        styleNode.setAttribute('type', 'text/css');
        styleNode.setAttribute('rel', 'stylesheet');
        if (sheet.media) {
            styleNode.setAttribute('media', sheet.media);
        }
        styleNode.id = id;

        if (!styleNode.styleSheet) {
            styleNode.setAttribute('href', 'data:text/css;base64,' + btoa(styles));


            // If new contents match contents of oldStyleNode, don't replace oldStyleNode
            keepOldStyleNode = (oldStyleNode !== null && oldStyleNode.getAttribute('href') === styleNode.getAttribute('href'));
        }

        var head = document.getElementsByTagName('head')[0];

        // If there is no oldStyleNode, just append; otherwise, only append if we need
        // to replace oldStyleNode with an updated stylesheet
        if (oldStyleNode === null || keepOldStyleNode === false) {
            var nextEl = sheet && sheet.nextSibling || null;
            if (nextEl) {
                nextEl.parentNode.insertBefore(styleNode, nextEl);
            } else {
                head.appendChild(styleNode);
            }
        }
        if (oldStyleNode && keepOldStyleNode === false) {
            oldStyleNode.parentNode.removeChild(oldStyleNode);
        }

        // For IE.
        // This needs to happen *after* the style element is added to the DOM, otherwise IE 7 and 8 may crash.
        // See http://social.msdn.microsoft.com/Forums/en-US/7e081b65-878a-4c22-8e68-c10d39c2ed32/internet-explorer-crashes-appending-style-element-to-head
        if (styleNode.styleSheet) {
            try {
                styleNode.styleSheet.cssText = styles;
            } catch (e) {
                throw new Error("Couldn't reassign styleSheet.cssText.");
            }
        }
    },

What's the problem with including the SourceMapGenerator module exactly? You've mentioned size, but in-browser compilation isn't really supported/recommended for production use anyway right?

Also note I've not tested any of this outside Chrome (and it will for sure fail in IE).

@jacquerie jacquerie referenced this issue in inveniosoftware/invenio Mar 24, 2015
Closed

assets: enable less source maps #2923

1 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment