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

QueryEngine not RequireJS compatible #18

Closed
aaronfay opened this issue Sep 6, 2012 · 4 comments
Closed

QueryEngine not RequireJS compatible #18

aaronfay opened this issue Sep 6, 2012 · 4 comments

Comments

@aaronfay
Copy link

aaronfay commented Sep 6, 2012

Hello,

When using the 'AMD' versions of Underscore and Backbone libraries, QueryEngine fails to load the proper dependencies using require("backbone"), etc.

As per this discussion require sync version needs to be used within a require or define callback, like so:

define(['foo-dep', function () {
  var foo = require('foo-dep');
});

The issue can be seen here

Patched version can be seen here

I apologize, I don't yet know coffeescript well enough to submit a pull request, but I have provided a diff of the javascript files if that helps. The main difference is a define wrapper at the top, and returning the global at the bottom. (code borrowed from knockout.js):

http://www.diffnow.com/?report=ignzq

Regards,
Aaron

@balupton
Copy link
Member

balupton commented Sep 7, 2012

Thanks for this. We'll need to come up with a different approach as QueryEngine is also made to run on the desktop side with Node.js, so window, document and navigator won't always be available.

Any ideas on something that supports AMD, No-AMD and Node?

@aaronfay
Copy link
Author

aaronfay commented Sep 7, 2012

Hello,

I borrowed the code from knockout.js, you can see the original sample here also had a condition for node/common.js modules:
https://github.com/SteveSanderson/knockout/blob/master/build/output/knockout-latest.debug.js

I simply removed the node.js branch thinking it wasn't necessary.

Also, instead of passing window to the factory function factory(window['ko'] = {}); I changed it to apply the context to window so it would be compatible with your existing setup, eg factory.call(window);.

Hth,
Aaron

@balupton
Copy link
Member

I'd love to support this. From experience however, maintaining compatibility with multiple module loaders is a real nightmare.

To use Query-Engine with Require/AMD/UMD, the build tools browserify and webpack have options to provide the necessary wrappers that you should need. It should also be fairly easy to accomplish this manually right?

Having this officially provided, also means that it has to be officially supported, which I simply cannot guarantee.

@balupton
Copy link
Member

Will be address by #43

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants