cujojs / curl

curl.js is small, fast, extensible module loader that handles AMD, CommonJS Modules/1.1, CSS, HTML/text, and legacy scripts.
https://github.com/cujojs/curl/wiki
Other
1.88k stars 216 forks source link

toUrl (called in js! plugin) doesn't work on dot path properly #184

Closed gehan closed 11 years ago

gehan commented 11 years ago

Inside the js! plugin around line 135, there is a call to require['toUrl'] that seems to mistake paths starting in . for something else. I think it messes up the pathRx in curl.js.

src/curl/plugin/js.js:136

name = order || exportsPos > 0 ? name.substr(0, name.indexOf('!')) : name;
// add extension afterwards so js!-specific path mappings don't need extension, too
url = nameWithExt(require['toUrl'](name), 'js');

If we log what's happening:

// If it starts like this it's fine
name = 'grunt/grunt-contrib-jasmine/reporter.js';
name = require['toUrl'](name);
// name = 'grunt/grunt-contrib-jasmine/reporter.js'

// However  a dot path throws it off completely
name = '.grunt/grunt-contrib-jasmine/reporter.js';
name = require['toUrl'](name);
// name = '.grunt/grunt-contrib-jasmine/.grunt/grunt-contrib-jasmine/reporter.js'
unscriptable commented 11 years ago

Hello @gehan,

I guess I hadn't considered the case where a module id could start with a ".". That's not allowed in CommonJS module naming (which AMD borrows).

I haven't set up a test case for this, so I'm just guessing... do you have a path (or package) mapping configured for ".grunt" or "grunt"?

-- J

gehan commented 11 years ago

I'm actually messing around with gruntjs, specifically trying to alter someone's requirejs jasmine spec runner to use curl instead of requirejs :D

The annoying thing is that when running that particular plugin it saves a number of scripts in .grunt, which need to be loaded!

Do you think adding a mapping might sort it out, or would 'grunt' be mapped back to '.grunt' before the above function would be called anyway?

The thing is if I remove js! and use curl without that plugin in then it's fine! Although obviously then doesn't run.

unscriptable commented 11 years ago

Sorry for not getting right back to you sooner, @gehan. Yes, a path map should fix the problem:

// for instance:
curl({
    paths: {
        'grunt-jasmine': '.grunt/grunt-contrib-jasmine'
    }
});
// later...
define(['js!grunt-jasmine/reporter.js'], function (reporter) { ... });

Did you try something like this?

-- J

gehan commented 11 years ago

No but I'll give it a go!

Is it possible to add to the paths options after curl has been loaded? ie. something like curl.paths['grunt-jasmine']

On 8 April 2013 19:42, John Hann notifications@github.com wrote:

Sorry for not getting right back to you sooner, @gehanhttps://github.com/gehan. Yes, a path map should fix the problem:

// for instance:curl({ paths: { 'grunt-jasmine': '.grunt/grunt-contrib-jasmine' }});

// later...define(['js!grunt-jasmine/reporter.js'], function (reporter) { ... });

Did you try something like this?

-- J

— Reply to this email directly or view it on GitHubhttps://github.com/cujojs/curl/issues/184#issuecomment-16069926 .

unscriptable commented 11 years ago

Yes, you can call curl({ paths: { /*...*/ } }) or curl.config({ paths: { /*...*/ } }) at any time. However, please note that calls to configure curl only affect modules fetched or defined in the future. Modules that have already been fetched or defined are not affected.

gehan commented 11 years ago

Yep that worked just fine thanks! I was altering a plugin so that I could use AMD modules as jasmine specs using curl as the AMD loader. Might be of interest to you as it means I now can do command linke headless js testing using grunt.

https://npmjs.org/package/grunt-template-jasmine-curljs

unscriptable commented 11 years ago

Nice! gonna check your node package since we're interested in testing with Jasmine.

gehan commented 11 years ago

Have a look here to see some specs set up with grunt/amd etc

https://github.com/gehan/bags/tree/dev-amd

There are some annoyances with promises though, look at this file which explains them and has a workaround (although with the q.js lib)

https://github.com/kriskowal/q/blob/master/spec/lib/jasmine-promise.js

I'd look at this too though. I think it's still quite early days but might be better! Has amd built in and is promise based apparently.

https://github.com/csnover/dojo2-teststack