yeoman / bower-requirejs

[DEPRECATED] Automagically wire-up installed Bower components into your RequireJS config
375 stars 68 forks source link

Respect formatting/code style #85

Closed mikkotikkanen closed 10 years ago

mikkotikkanen commented 10 years ago

Fe. if I have a long list of shims...

require.config({
    paths: {},
    shim: {
        module1: { deps: ["jquery"], exports: "$.fn.popover" },
        module2: { deps: ["jquery"], exports: "$.fn.popover" },
        module3: { deps: ["jquery"], exports: "$.fn.popover" },
        ...
    }

When I run bower-requirejs, this will be munged into...

require.config({
    paths: {},
    shim: {
        module1: {
            deps: [
                "jquery"
            ],
            exports: "$.fn.popover"
        },
        module2: {
            deps: [
                "jquery"
            ],
            exports: "$.fn.popover"
        },
        module3: {
            deps: [
                "jquery"
            ],
            exports: "$.fn.popover"
        },
        ...
    }

...which in this case then doesn't match the defined code style.

It would be awesome if previously shimmed modules are preset, to respect the formatting they might have.

I mean it doesn't even add shims so why reformat it to some arbitrary format?

eddiemonge commented 10 years ago

It seems like your example doesn't match a defined code style as you have two different styles going on there:

How is the module supposed to know which to use?

It also uses a parser to interpret the code and then output it again.

mikkotikkanen commented 10 years ago

Really? Closing the issue without even waiting for answer?

It does follow the code style. As you know, code style isn't necessarily something that is set to stone in everywhere, that is only for automated checking. In this case, long lists of similar small objects should be written as one-liners to keep the file as easy to understand as possible.

Regardless, the point is just to inject the paths and leave everything else as is instead of overwrititing/munging things willynilly. It does preserve the formatting elsewhere on the file, so it would make sense to keep the same formatting on the other parts of the config as well and only touch the paths object. Also, even if the injected paths would differ from the current formatting, that's lesser evil that reformatting the whole config as the module sees fit. Which can easily be quite long and would be PITA to reformat by hand.

sindresorhus commented 10 years ago

We get a massive amount of issues and it's easier to be trigger happy and reopen than not.

We use RequireJS to modify the config, so take it up on its issue tracker ;) https://github.com/yeoman/bower-requirejs/blob/4ae4689703fc66612b55f546a79889e70e2304d7/lib/index.js#L81

mikkotikkanen commented 10 years ago

Aight. Thanks for the constructive feedback. Will post the issue there.