welldone-software / gulp-durandal

Gulp plugin for building durandaljs projects
MIT License
13 stars 19 forks source link

Allow overriding/adding any requirejs parameter #1

Closed rockResolve closed 10 years ago

rockResolve commented 10 years ago

I want to be able to set the pragmas property on the rjsConfig object. But it seems a bit pointless to keep on adding gulp_durandal properties for every requireJs property anyone requests. (I messed around with quite a few properties before settling on pragmas).

So how about a global requirejs object which can override or add any requirejs property.

I would use like: durandal({ ... requirejs: { pragmas: { prodBuild: true }, } })

danikenan commented 10 years ago

I have pulled your change, but then made some changes of my own.

In any case, your input is accepted and the new version should allow plugin user to make changes to the rjs config object.

I added a options.rjsConfigAdapter parameter, which is an optional adapter function called just before the config object is passed to the optimiser. You can add/change any fields, and return the same object or a even new one.

I prefer this adapter fn to passing an object, since we are already out of the comfort zone of 'simple configuration' and using this adapter gives total control to advanced users, allowing them to fine-tune and complement the calculations that we do. Passing a 'dumb' object, created before our calculations and without a way to peek at them and use our config values, will require users who need to slightly change them to completely recalculate them/replace them, when they don't really need to so.

Would you please see if this approach works for you too.

Note, code has not yet been tested.... Please feel free to test and fix if it is not working or buggy.

In a few days, we will build our tests for this feature and once they pass, I will upgrade the version and also publish to npm.

rockResolve commented 10 years ago

I agree the adapter fn approach is a better approach. I've tested it on my code and it works fine.

danikenan commented 10 years ago

Done.

mryellow commented 9 years ago

Would be nice if options like generateSourceMaps and customisations to uglify2 config were passed through from the durandal constructor. Could hit all the stuff documented here then leave the function for anything out of the ordinary:

https://github.com/jrburke/r.js/blob/master/build/example.build.js

danikenan commented 9 years ago

Please look at options.rjsConfigAdapter.

On Thu, Feb 19, 2015 at 12:10 AM, mryellow notifications@github.com wrote:

Would be nice if options like generateSourceMaps and customisations to uglify2 config were passed through from the durandal constructor. Could hit all the stuff documented here then leave the function for anything out of the ordinary:

https://github.com/jrburke/r.js/blob/master/build/example.build.js

— Reply to this email directly or view it on GitHub https://github.com/welldone-software/gulp-durandal/issues/1#issuecomment-74959212 .

mryellow commented 9 years ago

Yeah it can do all that, but it would be more intuitive for these options to be passed through by default.

danikenan commented 9 years ago

Sorry, but you're missing the whole point of the plugin.

  1. The idea is to hide the complexity of rjs and provide something minimalistic, that fits 90% of the cases using 0-2 simple config options.
  2. The options we offer are in our own vocabulary, not that of rjs/requirejs.
  3. You propose to mix vocabularies. Mix rjs and none rjs options. This seems a bad idea, certainly not a "more intuitive" one.
  4. We want to promote the simplified usage, not the complex rjs usage.
  5. The plugin follows our motto: keep simple things simple, ensure complex things are possible.

In short, we prefer a simple usage and syntax for the 90% who are not familiar with complex rjs parameters, and make things intuitive for them. We enabled complete freedom with the config adapter. What more???

On Thu, Feb 19, 2015 at 6:23 AM, mryellow notifications@github.com wrote:

Yeah it can do all that, but it would be more intuitive for these options to be passed through by default.

— Reply to this email directly or view it on GitHub https://github.com/welldone-software/gulp-durandal/issues/1#issuecomment-74997331 .

mryellow commented 9 years ago

The keyword here is "intuitive", Grunt and other pieces that work with RequireJS often configure it by passing on the config objects on directly, this sets up a paradigm which means when arriving at gulp-durandal people can be a little misdirected when they see the options, as testimonied by this thread and others, multiple confused people.

Think it's just a tiny documentation issue.

When a new user sees the options available, it sets up a familiar picture "these all look like RequireJS options, everything else just passes them through, I'll just add some extra options like I've done before with other libraries".

function(rjsConfig){
  rjsConfig.pragmas = {
        fooExclude: true
  };
  return rjsConfig;
}

Fixing this example up to be a real example would be clearer.

function(cfg) {
        cfg.preserveLicenseComments = false;
        cfg.generateSourceMaps = false;
        cfg.uglify2 = {
            output: {
                beautify: false
            },
            compress: {
                sequences: false,
                global_defs: {
                    DEBUG: false
                },
                drop_console: true
            },
            warnings: true,
            mangle: false
        };

        return cfg;
    }
})

Could include as many of the RequireJS properties as possible, making it a template of sorts.

danikenan commented 9 years ago

We are not a requirejs plugin, but a Durandal one.

On Tue, Feb 24, 2015 at 11:51 PM, mryellow notifications@github.com wrote:

The keyword here is "intuitive", Grunt and other pieces that work with RequireJS often configure it by passing on the config objects on directly, this sets up a paradigm which means when arriving at gulp-durandal people can be a little misdirected when they see the options, as testimonied by this thread and others, multiple confused people.

Think it's just a tiny documentation issue.

When a new user sees the options available, it sets up a familiar picture "these all look like RequireJS options, everything else just passes them through, I'll just add some extra options like I've done before with other libraries".

function(rjsConfig){ rjsConfig.pragmas = { fooExclude: true }; return rjsConfig; }

Fixing this example up to be a real example would be clearer.

function(cfg) { cfg.preserveLicenseComments = false; cfg.generateSourceMaps = false; cfg.uglify2 = { output: { beautify: false }, compress: { sequences: false, global_defs: { DEBUG: false }, drop_console: true }, warnings: true, mangle: false };

    return cfg;
}

})

— Reply to this email directly or view it on GitHub https://github.com/welldone-software/gulp-durandal/issues/1#issuecomment-75854296 .

mryellow commented 9 years ago

Well guess the documentation doesn't need clearing up then, no issue and no one has ever been confused by it. That example would make this much clearer, but too much hard work I guess.

p.s. Durandal = jQuery + RequireJS + KnockoutJS, says it right on the tin.

Take it or leave it mate, the docs can be improved or not. I don't really care. Thanks for the attitude.

danikenan commented 9 years ago

We have ~ 1000 d/l per month, for more than a year. No one else complained...

On Wed, Feb 25, 2015 at 5:35 AM, mryellow notifications@github.com wrote:

Well guess the documentation doesn't need clearing up then, no issue and no one has ever been confused by it.

— Reply to this email directly or view it on GitHub https://github.com/welldone-software/gulp-durandal/issues/1#issuecomment-75899928 .

mryellow commented 9 years ago

Righto... Writing clearer docs that better explain your systems is never a good idea.

Back to counting downloads. It's ok, I'm a bit of an arrogant prick myself, as I'm sure you can tell. Many of us are sometimes it seems.

danikenan commented 9 years ago

I fact, we added quite a few features requested by others and even incorporated several pull requests of code and changes.

We simply do not agree with you on the features you wanted at beginning, as we find them promoting bad practices (mixing vocabularies as well as not hiding the rjs complexity). Also, we do not agree the intuitiveness of you request, we find it contra-intuitive.

This is our view, like it or not.

We feel that your expectations are based on your familiarity with other requirejs and durandal plugins in grunt, which we found wrong to begin with (that is why we created our own).

Never the less, you are more than welcome to propose an improvement to the docs that will help people like you (with the same state of mind and expectations) better understand how to use this plugin. If its good, rest assure we add you doc improvements.

On Wed, Feb 25, 2015 at 6:49 AM, mryellow notifications@github.com wrote:

Righto... Writing clearer docs that better explain your systems is never a good idea.

Back to counting downloads.

— Reply to this email directly or view it on GitHub https://github.com/welldone-software/gulp-durandal/issues/1#issuecomment-75905149 .

mryellow commented 9 years ago

You made the mixed vocab point well and I agree.

At which stage it became a documentation issue... but it's ok, I've had a cookie now. ;-)