output of r.js - incorrect sequence of concatenated output #1342

Closed
ghost opened this Issue May 8, 2015 · 11 comments

Projects

None yet

1 participant

@ghost
ghost commented May 8, 2015

Using version 2.1.11 :

I have a project that uses the r.js optimizer. The optimized output is a webapp. When I run the webapp in the browser I am getting a reference error; in particular 'jQuery is not defined'.

I can say that most of the modules are dependent on jQuery. So naturally, I would expect that jQuery be the first 'module' that is in the concatt'ed output. However, it is actually the 2nd module. The first module is a module that depends on jQuery. (it happens to be a jquery plugin - select2)

I have specified a shim in the shim config like so:

'select2': {
            deps: ['jquery'],
            exports: 'Select2'
}

One thing I should mention. When I manually edit the output file - as in - cut out the jquery block and move it to the top of the file, everything works as expected.

My question is:

How is the sequence of the output in the optimized file determined? How can I specify the sequence?

@jrburke
Member
jrburke commented May 10, 2015

In general, it should put the jquery dependency first. My first guess is that the r.js optimizer is not aware of the shim config.

Are you passing the shim config to the r.js build tool (it needs to know it to properly do the build)? If so, how? If via mainConfigFile, is it the first requirejs config call in that file?

@ghost
ghost commented May 10, 2015

I am using r.js via grunt - as the grunt-contrib-requirejs module.

I had suspected that this may be the case - so what I tried was changing the reference to jquery in the shim config for select2. ie:

'select2': {
            deps: ['jquer$$$y'],
            exports: 'Select2'
}

The reasoning was that if r.js is not getting the shim config, then i should be able to put any string in the dependency and it should have no effect on the build process.

Doing so broke the r.js 'Tracing dependencies for: main' process. It essentially acknowledged that 'path\To\jquer$$$y' is not a directory or file.

So I can say with some certainty that the shim config is being processed by r.js

What do you think? Any other leads here? I can provide you with more info if needed. Thank you very much for you help.

@jrburke
Member
jrburke commented May 10, 2015

If it is something wrong with the select2 file in particular, maybe pointing to that and I can see if a simple r.js build with just that shim config works.

@ghost
ghost commented May 11, 2015

Sure, the version is the latest (4.0.0). Here is the url to lib:

https://github.com/select2/select2

Please let me know if you need more info. It is dependent on jQuery. We are using jQ ver. : 2.0.3

Again -thanks for the help; I really appreciate your responsiveness.

@jrburke jrburke added this to the 2.1.18 milestone May 13, 2015
@jrburke
Member
jrburke commented May 13, 2015

Thanks for the pointer. This is a variation of requirejs/r.js#808, where r.js is thinking this module defines an AMD API, so it hoists the file to the top. While it does define an AMD API, it is local to the internals of the file, not publicly visible. So a fix in r.js needs to be made to ignore this local define.

I'm putting this in the 2.1.18 bucket. I am not sure yet when 2.1.18 will come out yet. I would say on the order of weeks instead of months, but subject to other factors. If you need a fix in the short term, you could use the onBuildRead and onBuildWrite hooks to trick the r.js optimizer. Something like this might work:

{
  onBuildRead: function (id, path, contents) {
    if (id === 'select2') {
      return contents.replace(/define.amd\s*=\s*\{/, 'define.amdTEMP = {'); 
    }
  },

  onBuildWrite: function (id, path, contents) {
    if (id === 'select2') {
      return contents.replace(/define.amdTEMP\s*=\s*\{/, 'define.amd = {'); 
    }
  }
}
@ghost
ghost commented May 13, 2015

Thanks again for looking into my issue. I will try out the 'work around' and update this thread with the results.

@ghost
ghost commented May 13, 2015

Hi - I really hate to have to bring this up, but I'm having an issue with the work around itself.

Here's the message - any ideas?

Tracing dependencies for: main
{ [Error: TypeError: Cannot call method 'indexOf' of undefined
    at Object.pragma.process (/path/To/node_modules/grunt-contrib-requirejs/node_modules/requirejs/bin/r.js:24224:51)
]
  originalError: 
   { [TypeError: Cannot call method 'indexOf' of undefined]
     fileName: '/path/To/main.js' } }

I placed the onBuildRead and onBuildWrite functions in the requirejs grunt task - options stanza like so:

requirejs: {
      ugly: {
        options: {
             ...,
             onBuildRead: function (id, path, contents) {
                   if (id === 'select2') {
                      return contents.replace(/define.amd\s*=\s*\{/, 'define.amdTEMP = {'); 
                  }
          },
          onBuildWrite: function (id, path, contents) {
              if (id === 'select2') {
                return contents.replace(/define.amdTEMP\s*=\s*\{/, 'define.amd = {'); 
              }
          }

Any ideas?

@ghost
ghost commented May 13, 2015

In r.js, on the lines specifed in the error message, I placed some debug output statements, and found that the 'fileContents' variable is 'undefined' and the 'fileName' is pointing to the main.js config file and not the 'select2' file - which is what I expected.

Is this symptomatic of anything in particular?

Hmm..should there be an 'else' block too on the 'onBuildRead/Write' overrides?

@ghost
ghost commented May 13, 2015

Yes there should:

the logic should be:

          onBuildRead: function (id, path, contents) {
              if (id === 'select2') {
                return contents.replace(/define.amd\s*=\s*\{/, 'define.amdTEMP = {'); 
              }else{
                return contents;
              }
          },
          onBuildWrite: function (id, path, contents) {
              if (id === 'select2') {
                return contents.replace(/define.amdTEMP\s*=\s*\{/, 'define.amd = {'); 
              }else{
                return contents;
              }
          }
@jrburke
Member
jrburke commented May 13, 2015

Ah, right, sorry about that, forgot to make sure the other module's contents are returned! That last one looks good.

@kevin-brown kevin-brown referenced this issue in select2/select2 Jan 20, 2016
Closed

Almond inside select2.js #3372

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