requirejs / almond

A minimal AMD API implementation for use after optimized builds
Other
2.42k stars 168 forks source link

Map config is ignored? #83

Closed adrian-chang closed 10 years ago

adrian-chang commented 10 years ago

Hey there,

When using almond and r.js for a build it seems that a map config options for "*" are ignored.

Example would be

map: { "*": { 'underscore': 'lodash' } }

The resulting built file using r.js and almond contains no stub of 'underscore'. Is this correct? I'd expect at least for "*" configs to create define stubs for the modules...

jrburke commented 10 years ago

There are no stub modules written for map config, rather, almond assumes the same config is included in the built file, and the ID resolution in almond uses the map config in the config.

Closing as a discussion ticket, but feel free to continue discussion here, and we can reopen if we find an issue.

givankin commented 10 years ago

Hi, continuing the discussion :)

almond assumes the same config is included in the built file

First, in my experience this is very unintuitive (and I've seen other developers struggling with it too). It took me long to realize that my map config is not broken, it's just not included into built file.

Second, it is not convenient in case with multiple builds, where map is used to exclude some files from a particular build. Instead of having a few options in a build script, one has to have multiple "main" files (at least I didn't find another way).

How costly would it be to fix that? And what kind of fix would it be? Maybe r.js should merge the final config and include into built file?

If the fix is undesirable for some reasons, at least this limitation should be clearly stated in documentation IMO.

jonchay commented 10 years ago

Hi there, I'm having the same issue as well. I need jQuery.noConflict and followed the instructions on RequireJS map config: http://requirejs.org/docs/jquery.html. But I realize that my jQuery isn't calling noConflict due to map config being ignored.

Is there a fix or workaround?

jrburke commented 10 years ago

Almond is really basic, just supports one requirejs.config call, and that config needs to be in the build output for map to work. I updated the readme to indicate this, and to also mention AMDclean as an alternative solution.

As for doing multiple builds that reuse a config see this gist. With that approach, the common config could specify a mainConfigFile whose file is also included in the output and that config has the map.

givankin commented 9 years ago

Let me follow up on this (after just hitting the issue again).

First, thank you @jrburke for updating the readme. It has made my reminding path shorter :-) (and I'm sure has helped others as well).

I was just thinking: if it's possible for r.js to have an option to include map configuration into out automatically (something similar to insertRequire option)? In the gist above I see that multiple configs are include-ed (main-frame, main-mods, main-edit), and this exactly what I want to avoid.

The case is that I have a significant "basic" map configuration which is extended a little bit for particular build type. And having to have map config in outer files means that I have to keep multiple files to keep (1 per build type), and content of those files is ~80-90% duplicated. Instead, I would like to keep map config in Gruntfile.js (we use grunt-contrib-requirejs), where I can avoid duplication just by using extend(basicMapConfig, specificMapConfig1).

Hope this makes sense =)

jrburke commented 9 years ago

You could keep the map in a file that has it wrapped in a requirejs.config(), include it in build config and for the grunt reuse, fs.readFileSync the file, regexp off the the config call then JSON.parse the result.

There can be multiple requirejs.config calls in a built file, just best to make sure they are all near the top of the file.

givankin commented 9 years ago

Thanks for the tips, @jrburke. Multiple requirejs.config calls is a nice one I didn't know about. It will allow to at least avoid duplication in config files.

So you think adding extra option for this case to r.js won't make sense?