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

proper handling for pragmas.excludeJadeOnSave #8

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

bendi
Copy link

@bendi bendi commented Oct 18, 2012

Added proper handling of pragmas.excludeJadeOnSave, thus enabling removal of runtime dependency to jade libraries. Checked against latest requirejs - works like charm :)

@vincentmac
Copy link
Collaborator

I'm going to have to take a closer look at this. One thing that concerns me are the updates you made directly to the Jade compiler and runtime. I would rather not have any changes to TJ's code except the last line in the compiler (around line 3648) that exports jade to the scoped Jade variable rather than a global window.jade variable.

Changing TJ's code will break or greatly limit the future upgradability of this module.

I'll try to make some time to play with your changes and learn a little more about using r.js to minify an app.

@bendi
Copy link
Author

bendi commented Oct 19, 2012

I was just following documentation - it mentions exclueJadeOnSave but I couldn't get it working. So I figured that maybe it's missing.

I also saw previous pull-merge that got rid of the local 'jade' variable completely, which in my opinion will break at runtime because of line 1033, which makes heavy use of it, inside pre-processed templates.

@vincentmac
Copy link
Collaborator

I finally got an opportunity to minify my source for a project. I did not run into any problems in the process. I'm using yeoman, which in turn uses grunt, to run r.js. In my r.js configuration section, I included the pragmasOnSave option and did not run into any errors.

Here's the relevant section from my Gruntfile.js:

    // rjs configuration. You don't necessarily need to specify the typical
    // `path` configuration, the rjs task will parse these values from your
    // main module, using http://requirejs.org/docs/optimization.html#mainConfigFile
    //
    // name / out / mainConfig file should be used. You can let it blank if
    // you're using usemin-handler to parse rjs config from markup (default
    // setup)
    rjs: {
      // no minification, is done by the min task
      // optimize: 'none',
      optimize: 'uglify',
      baseUrl: './javascripts',
      wrap: true,
      name: 'main',
      pragmasOnSave: {
        excludeJade : true
      },
      paths: {
        backbone: '../components/backbone/backbone-min'
      , bootstrap: '../components/bootstrap/bootstrap/js/bootstrap-min'
      , jade: '../components/require-jade/jade'
      , jquery: '../components/jquery/jquery.min'
      , underscore: '../components/underscore/underscore-min'
      , 'backbone.paginator': '../components/backbone.paginator/dist/backbone.paginator.min'
      },
      out: 'lessons.min.js'
    },

I first created a non-minified version to verify what was in there. As expected, the Jade Compiler is not included in the source. Instead, all jade templates get compiled to javascript functions and the Jade Runtime is included to allow the application to interact with them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants