Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Ensure empty config is a live reference #532

Open
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Contributor

guybedford commented Nov 17, 2012

This is a first commit towards achieving jrburke/require-cs#35

The 'createXhr' call for the text plugin works perfectly for these needs.

The only issue is that if no config is specified, the text plugin gets an empty object for config which isn't referenced on the original config object. This means that a dynamic config change on that config object won't be seen by the text plugin.

This pull request ensures that when a module asks for config and there is none, the config is set to an empty object which is properly referenced on the config object to ensure future changes reference through correctly.

Owner

jrburke commented Nov 20, 2012

I looked more into this, and it also then implies merging module config calls. This is the patch I came up with:

diff --git a/require.js b/require.js
index b60d954..5433eae 100644
--- a/require.js
+++ b/require.js
@@ -566,7 +566,12 @@ var requirejs, require, define;
                         id: mod.map.id,
                         uri: mod.map.url,
                         config: function () {
-                            return (config.config && getOwn(config.config, mod.map.id)) || {};
+                            var id = mod.map.id,
+                                cfg = getOwn(config.config, id);
+                            if (!cfg) {
+                                cfg = config.config[id] = {};
+                            }
+                            return cfg;
                         },
                         exports: defined[mod.map.id]
                     });
@@ -1231,6 +1236,14 @@ var requirejs, require, define;
                     if (objs[prop]) {
                         if (prop === 'map') {
                             mixin(config[prop], value, true, true);
+                        } else if (prop === 'config') {
+                            eachProp(value, function (v, p) {
+                                var c = config.config;
+                                if (!c[p]) {
+                                    c[p] = {};
+                                }
+                                mixin(c[p], v, true);
+                            });
                         } else {
                             mixin(config[prop], value, true);
                         }

And, test:

        require({
            config: {
                a: {
                    id: 'magic'
                }
            }
        });

        define('a', function (require, exports, module) {
            var config = module.config();

            return function () {
                return config.color;
            }
        });

        require(['a'], function(a) {
            var second,
                first = a();

            require.config({
                config: {
                    a: {
                        color: 'red'
                    }
                }
            });
            second = a();

            doh.register(
                'moduleConfigDelayed',
                [
                    function moduleConfig(t){
                        t.is(false, !!first);
                        t.is('red', second);
                    }
                ]
            );
            doh.run();
        });

But it starts to fall down when I look at something like almond -- which assumes there is only one requirejs.config call, normally done before any modules are traced.

So this makes me think that this sort of thing is only useful for define()d modules that then call requirejs.config(), which is something I want to discourage -- declaring a module should be separate from setting config, except for maybe the top level module for the app.

So I'm on the fence about this. Leaving open though for more consideration later.

Contributor

guybedford commented Nov 21, 2012

Thanks for trialling this. Yes it's not meant to encourage live rerunning of config within modules. Rather to have the ability for modules to communicate with each other through config, by maintaining reliable live references to each other's config. The main issue here being that the live reference is lost if a module is instantiated without config.

Admittedly, the means of locating and live updating another modules config doesn't exist without dipping into the private api.

Perhaps an API like:

define('mymodule', ['module'], function(module) {
  var textConfig = module.getConfig('text');
});

explains the context of the system a bit better? Dynamic changes being made through object reference rather than require.config calls.

My access method was actually going to be along the lines of requirejs.s.contexts[contextName].config.config.text.createXhr = syncXhr and so not almond compatible anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment