RaveJS / rave

Zero-configuration application bootstrap and development
278 stars 8 forks source link

AMD require(array, callback) is busted #2

Closed unscriptable closed 10 years ago

unscriptable commented 10 years ago

require(array, callback) passes through to require.ensure/require.async, which doesn't take an array for the first parameter. Should it?

unscriptable commented 10 years ago

Ok. The old, pre-promises CommonJS proposed addition is require.ensure(array, callback). Inject follows this.

Montage Require specifies require.async(id): Promise.

We are implementing both as require.async(id): Promise and require.ensure(id): Promise. This is just going to confuse folks, obviously.

I like the returned promise much better, but we lose the ability to require multiple modules at once.

Maybe we should stick with require.async(id): Promise, remove our non-conforming require.ensure, and implement AMD's require(array, callback, errback) like this:

    function amdRequire (id, callback, errback) {
        if (id in scopedVars) return scopedVars[id];

        if (arguments.length > 1) {
            if (typeof id === 'string') {
                return cjsRequire.async(id).then(callback, errback);
            }
            else {
                // code to map ids over cjsRequire.async and spread onto callback
            }
        }
        else {
            return cjsRequire(id);
        }
    }
unscriptable commented 10 years ago

I ended up implementing amdRequire like this:

    function amdRequire (id, callback, errback) {
        if (typeof id === 'string') {
            return requireSync(id);
        }
        else {
            return Promises.all(ids.map(requireOne))
                .then(callback, errback);
        }
    }

    function requireSync (id) {
        return id in scopedVars
            ? scopedVars[id]
            : cjsRequire(id);
    }

    function requireOne (id) {
        return id in scopedVars
            ? scopedVars[id]
            : cjsRequire.async(id);
    }
unscriptable commented 10 years ago

Fixed in 495dc09