Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Built app (debug/release): some required libraries resolving as undefined #185

Closed
fiznool opened this issue Dec 18, 2012 · 16 comments
Closed

Comments

@fiznool
Copy link
Contributor

fiznool commented Dec 18, 2012

It seems as though some of the included libraries (Backbone, Lodash) aren't being required properly in built mode (debug or release) when added as dependencies to modules.

To reproduce:

Modify main.js as follows:

require([
  // libs
  "jquery",
  "lodash",
  "backbone",
  // Application.
  "app",

  // Main Router.
  "router"
],

function($, _, Backbone, app, Router) {

  console.log(arguments);

  ...

Run the build tools: bbb debug

Fire up the local server: bbb server:debug

The console will look like the following:

Screen Shot 2012-12-18 at 17 28 49

Notice that jQuery has resolved OK (first argument) whereas lodash and backbone have not. Could it be that shimmed libraries are not resolving correctly with the almond.js loader?

This works fine when in regular mode (bbb server).

fiznool added a commit to fiznool/mobile-backbone-boilerplate that referenced this issue Dec 18, 2012
@fiznool
Copy link
Contributor Author

fiznool commented Dec 20, 2012

Some more investigation today.

Tried replacing the almond.js shim with the full require.js script, this did not change anything.

It looks like the combination of r.js and jam.js has created a double definition of backbone and backbone.layoutmanager. In the built debug script:

jamjs-require

Lines 7477 and 8266 look a little strange?

@fiznool
Copy link
Contributor Author

fiznool commented Dec 20, 2012

Including @caolan in case he can help. :)

@austinhappel
Copy link

I'm having the same issue. A workaround is to place backbone as a dependency in new modules, but not pass it as an argument to the module function. In this way, requirejs ensures that backbone.js has been executed before your module - and you instead use the global Backbone variable instead of passing it in with local scope. This seems to be reliable, but it isn't a fix to the real problem and sort of defeats the purpose of AMD. I'm hoping this will serve to help others in the meantime until this gets fixed.

Here's a super basic module example explaining the problem:

// Testing module
define([
  "backbone" // ensures backbone is loaded
],

function(Backbone) {
  console.log(window.Backbone); // works.
  console.log(Backbone); // returns undefined.
});

So the workaround is to just not include Backbone as an argument:

// Testing module
define([
  "backbone" // ensures backbone is loaded
],

function() {
  console.log(Backbone); // global scope: shows window.Backbone, which works.
});

@SBoudrias
Copy link
Contributor

If you're loading your app directly:

<script src="path/to/build.js"></script>
<!-- Instead of preivously -->
<script data-main="app/config" src="assets/js/libs/require.js"></script> 

You're probably breaking your path (and so your module name).

As a workaround, you should add this into the grunt.js requirejs task

wrap: {
    // set baseUrl config if concat is used
    start: 'require.config({ baseUrl: "app/", waitSeconds: 30 });',
    end: ' '
}

If that's not the problem, try posting your grunt.js requirejs task config so we can have a look.

Also, have you made sure that Backbone.js is correctly included into the builded file?

@austinhappel
Copy link

@SBoudrias I tried playing around with paths as you suggested, but that did not work. I made a gist of all pertinent files in my setup (it's a default bbb project with minor modifications) here: https://gist.github.com/4478229

Steps to reproduce this issue:

  1. Make a test directory, initialize bbb, then make a module:
    bash mkdir ~/Desktop/test && cd ~/Desktop/test && bbb init && bbb init:module
  2. At the module prompt, name your module 'testing'
  3. Replace modules/testing.js with the file in gist (link above)
  4. Add 'modules/testing' to the list of dependencies in app.js (see gist)
  5. in console, run bbb debug && bbb server:debug
  6. open site in browser
  7. See that console.log outputs undefined (Backbone is undefined)
  8. in console, kill server then run bbb server
  9. refresh site in browser
  10. See that console.log outputs Backbone object (no problems when not concatenated)

Hope that helps. And thanks for offering to help!

@SBoudrias
Copy link
Contributor

The example works with Require.js, but not with almond. That's because Jamjs is using packagesPath config, but those aren't supported by Almond.

So either don't use Jam or don't use Almond.

You can always ask on Almond repo if they intend on implementeing packagesPath config in the future

@tbranyen
Copy link
Owner

tbranyen commented Jan 7, 2013

Hrm I'm using it just fine in Almond. Something else must be messed up. I'll look into it tonight.

@austinhappel
Copy link

I also want to comment: I tried replacing almond.js with require.js (line ~88 in the grunt.js file) and tested again: just like @fiznool, it did not fix the problem.

@SBoudrias
Copy link
Contributor

Well, I've tested your gist, and it was to working builded with require.js in place of almond on my end.

But, about your code, there's some things you can fix:

app.js is mostly used for plugin configuration and global event communication. You shouldn't bootstrap your app by requesting your modules there. Require your module into main.js.

A thing that I can see is that require.js is asynchrounously loading modules, and it may load main.js before the jamjs config (or concat it before, etc, in the builded file). You may want to try to make sure jam/require.config.js is loaded fist:

require([
    "../vendor/jam/require.config"
], function() {

    require.config({

      // Initialize the application with the main application file and the JamJS
      // generated configuration file.
      deps: [ "main"],

      paths: {
        // Put paths here.
      },

      shim: {
        // Put shims here.
      }

    });

});

Although I don't think this could really be a problem in the builded version

@svperfecta
Copy link

Just a note, that I'm having a similar issue. Steps above reproduce mine as well.

@tbranyen
Copy link
Owner

tbranyen commented Jan 9, 2013

Okay, guess we have a few options here:

  • Keep JamJS and remove Almond.
  • Remove JamJS and keep Almond.

@jrburke could you weigh in on this? I'd hate to lose either, but it looks like Almond needs to support package configurations better.

@jrburke
Copy link

jrburke commented Jan 9, 2013

@tbranyen almond 0.2.2 has a fix related to packages, and the r.js optimizer master has a different change related to inlining modules from packages. I'm in the process of fixing issues for a requirejs 2.1.3 release, and that optimizer change will be part of that. You can try it now by using the master r.js snapshot to see if that helps.

If you can confirm that either almond 0.2.2 on its own fixes it, or if it also needs what is in the master r.js, that would be good to know. Or if neither helps. There is also this change for the optimizer that I have slotted to evaluate for 2.1.3. Not sure if this is related to shim config use though.

The general design goal for almond/r.js: a built/optimized library that uses almond should not need package config since all the modules have been inlined with the right normalized module ID names. There could be a bug with that approach, but I suspect it will be more of an r.js fix vs. an almond fix.

@jrburke
Copy link

jrburke commented Jan 9, 2013

I just merged requirejs/r.js#331 in master, so if you grab an r.js master snapshot that is dated this or later:

2.1.2+ Wed, 09 Jan 2013 22:28:38 GMT

then that contains the 331 fix related to packages and shim config in building.

@austinhappel
Copy link

Success! Using the setup I previously described I tried upgrading almond first (pulled latest from the git repo - commit: ccc964b30f) - but this did not solve the issue.

Next, I swapped out the default r.js that bbb uses with the snapshot @jrburke listed above, which has solved the issue. Backbone is no longer returning as undefined.

So in conclusion: The update to r.js fixed the issue, but the almond.js update did not. It would be good if someone else could confirm this. Next question: So then, is the solution to just wait until this updated version of r.js gets into the npm registry, and patch it in the meantime, or is this something that can be configured via the bbb package file? (I'm a newbie when it comes to node packages)

Thanks again for help guys!

steps to repro success:

  1. Create a test bbb app by following steps 1-4 from this comment
  2. Assuming you have bbb installed globally (via npm install -g), replace the r.js file found here: /usr/local/lib/node_modules/bbb/node_modules/requirejs/bin/r.js with the one from the previous comment
  3. back in your test project run bbb debug && bbb server:debug
  4. in the browser console, backbone will now return correctly, and not be undefined

@tbranyen
Copy link
Owner

tbranyen commented Jan 9, 2013

Awesome, I'll wait until @jrburke pushes a release and upgrade everything. Thanks very much for your help @austinhappel =)

@fiznool
Copy link
Contributor Author

fiznool commented Jan 10, 2013

I can also confirm the fix of swapping out the r.js file with the one in the snapshot @jrburke listed above works for me too.

Just a small difference, on my machine (OSX Mountain Lion) the r.js file that needs to be modified is located at /usr/local/share/npm/lib/node_modules/bbb/node_modules/requirejs/bin/r.js.

Thanks from me too!

@tbranyen tbranyen closed this as completed Apr 8, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants