mariusGundersen / gulp-amd-optimizer

Emits the entire dependency tree of one or more AMD modules, from leaves to root, and names anonymous modules
22 stars 4 forks source link

Relative module ids #1

Open jeffrose opened 10 years ago

jeffrose commented 10 years ago

It appears that relative module ids are miscalculated.

Using (this mildly contrived example)...

gulp.src( 'src/*.js' ) )
    .pipe( amdOptimizer( {
        baseUrl: 'src'
    } ) )
    .pipe( concat( 'foobar.js' ) )
    .pipe( gulp.dest( 'dist' ) );

... with the following modules...

// /src/my/foo
define( function(){
    // Do foo
} )

// /src/my/other/bar
define( [ '../foo' ], function( foo ){
    // Use foo to do bar
} )

... results in...

Error: ENOENT, no such file or directory '/foobar/src/../foo.js'

Using ., e.g. ./other/bar, results in a similar error.

mariusGundersen commented 10 years ago

Thanks for the feedbak. This is a problem, and is related to the map config, which does not work either.

The problem is that the require project does not expose a way to find one module relative to another module. It only exposes the toUrl method, which finds the path to a module using the name of the module. Unfortunately it does not take the depending module as a second parameter, and so relative url's do not work.

One solution to this is to get the require project to expose a toUrl method which can take two module names as parameters. Another solution is to re-implement the entire module finding algorithm from requireJS. The second option, I worry, will lead to bugs and edge case differences between requireJS and this project.

jeffrose commented 10 years ago

Thanks for the response.

@jrburke might be able to provide some guidance. I'm sure it's seen as a rare use case, but exposing the API might be trivial.

jrburke commented 10 years ago

I agree this needs to be easier to use. Right now, it is exposed in the default loader context via requirejs.s.contexts._.makeModuleMap, which is defined here: makeModuleMap.

For this sort of use, passing false for isNormalized and true for applyMap seem to make sense.

That could be used for now. While I do not guarantee API stability for those methods inside those context objects, in reality they have been fairly stable, and any rewrites I do will likely be a larger version update where I try to align with ES module loader APIs.

I filed jrburke/requirejs#1120 to track a better in the loader.

mariusGundersen commented 10 years ago

I've been looking into this again (among other things) and one problem with relative urls is that it can lead to duplicate content. If two modules depend on the same common lib, but have two different (relative) paths to it, then the lib would need to be included in the optimized output twice. This isn't very optimal.

If it does include the same module twice (with different names) then the only reasonable thing is to warn the programmer about it.