concordusapps / grunt-haml

Process HAML templates to precompiled JavaScript or rendered HTML.
https://npmjs.org/package/grunt-haml
MIT License
41 stars 28 forks source link

Lines following a -# comment within a <script> tag disappear after upgrade to 0.6.1 #19

Closed paulkoegel closed 10 years ago

paulkoegel commented 11 years ago

HAML code:

  %body
    %script{type: 'text/javascript'}
      BB.preloadedData = {};
      -# do NOT change the following two lines as we're replacing them via a regex in the insert_json_into_dom grunt task
      BB.preloadedData.about = [];
      BB.preloadedData.projects = [];

relevant output with 0.6.1 and 0.8.0 (lacking "about" and "projects"):

    <script type='text/javascript'>BB.preloadedData = {};
    </script>

relevant output with 0.6.0:

<script type='text/javascript'>BB.preloadedData = {};
      BB.preloadedData.about = [];
      BB.preloadedData.projects = [];
</script>

The comment is NOT supposed to appear in the rendered HTML. Am I doing something wrong and is this intended behaviour inside <script> tags?

joneshf commented 11 years ago

Well, the only thing that happened from 0.6.0 to 0.6.1 was updating haml-coffee from 1.10.x to 1.11.x. So my first guess would be to look there.

As a cursory look, I'd guess it might be related to netzpirat/haml-coffee#67. In particular, this line.

Of course, this is all just a guess, and would require actual diag to figure out the issue.

paulkoegel commented 11 years ago

thanks for your reply. summoning @netzpirat ;)

netzpirat commented 11 years ago

I can confirm the problem. Until fixed, you can switch to a filter for the javascript:

%body
  :javascript
    BB.preloadedData = {};
    // do NOT change the following two lines as we're replacing them via a regex in the insert_json_into_dom grunt task
    BB.preloadedData.about = [];
    BB.preloadedData.projects = [];

which works just fine. I'll keep you updated...

netzpirat commented 11 years ago

Haml-Coffee 1.13.1 released which fixes the issue. Thanks for the failing spec :)

joneshf commented 11 years ago

@netzpirat Thanks for the timely fix!

@paulwittmann Can you check this and ensure it works for you now?

paulkoegel commented 11 years ago

hm, still getting:

<script type='text/javascript'>BB.preloadedData = {};
    </script>

grunt-haml 0.8.0 and haml-coffee 1.13.1 now.

$ npm list
├─┬ grunt-haml@0.8.0
│ ├── haml@0.4.3
│ └─┬ haml-coffee@1.13.1
netzpirat commented 11 years ago

Oops, my spec omitted the {type: 'text/javascript'} (HTML5 FTW), so I didn't catch another issue where the attribute lookahead swallows the next line. This is fixed now in version 1.13.2

joneshf commented 11 years ago

@paulwittmann What's the word?

paulkoegel commented 11 years ago

Works now. Thanks a lot! (haml-coffee 1.13.3, grunt-haml 0.8.0)

paulkoegel commented 11 years ago

Sorry, now there's another issue:

%script{type: 'text/javascript'}
  -# our app's namespace
  window.BB = {};
  BB.env = 'development';
  -# live reload
  document.write('<script src=\"http://' + (location.host || 'localhost').split(':')[0] + ':35729/livereload.js?snipver=1\"><\\/script>');

  WebFontConfig = {
    google: { families: [ 'Roboto:400,100,500,500italic,300italic,300,700:latin', 'Roboto+Condensed:400,700:latin' ] }
  };
  (function() {
    var wf = document.createElement('script');
    wf.src = ('https:' == document.location.protocol ? 'https' : 'http') +
      '://ajax.googleapis.com/ajax/libs/webfont/1/webfont.js';
    wf.type = 'text/javascript';
    wf.async = 'true';
    var s = document.getElementsByTagName('script')[0];
    s.parentNode.insertBefore(wf, s);
  })();

is converted to:

 <script type='text/javascript'>
      WebFontConfig = {
        google: { families: [ 'Roboto:400,100,500,500italic,300italic,300,700:latin', 'Roboto+Condensed:400,700:latin' ] }
      };
      (function() {
        var wf = document.createElement('script');
        wf.src = ('https:' == document.location.protocol ? 'https' : 'http') +
          '://ajax.googleapis.com/ajax/libs/webfont/1/webfont.js';
        wf.type = 'text/javascript';
        wf.async = 'true';
        var s = document.getElementsByTagName('script')[0];
        s.parentNode.insertBefore(wf, s);
      })();
    </script>

works when replacing <script> with :javascript (where I have to use // and which includes my comments in the compilation) except then I get a plain <script> tag in the HTML without a type attribute.

joneshf commented 11 years ago

I'm not able to test this at the moment, so, can you try with a slightly smaller example? Like:

%script{type: 'text/javascript'}
  var x = 0
  -# Some comment line
  var y = 1

Maybe we can get to the bottom of this.

joneshf commented 10 years ago

I'm going to assume this is good.