AMD version does not work in a node-webkit envrionment

[dogawaf]

Hi

Somebody tried to use simple-auth in a amber-cli/node-webkit application.

But AMD version of simple-auth looks for require in global, then in window.
https://github.com/simplabs/ember-simple-auth/blob/master/packages/ember-simple-auth/wrap/amd.end#L1

This has the drawback of using node's require instead of browser's require, and throws a nice Uncaught Error: Cannot find module 'ember'.

I don't know very much of AMD, but do we have another option than looking for require in global?

[marcoow]

Huh, I'm not really an AMD expert neither - guess we need help from someone else here ;)

[dogawaf]

Maybe @rwjblue is an AMD expert? ;)

[marcoow]

I'm pretty sure he is ;)

[rwjblue]

Lord help us all...

[dogawaf]

Does simple-auth should support node? I think it is aimed to run only in the browser, isn'it?

[marcoow]

In general it's meant to run in Ember apps so if an Ember App runs in node with node-webkit then Ember Simple Auth would want to work as well...

However, I never really used node (not to mention Ember + node-webkit) so I don't know much about how that would work.

[dogawaf]

In a node-webkit app, Ember runs in the browser (chromium).
So I think the AMD wrapper that simple-auth uses should be simplified to just use the global define or require, the same that Ember defines and uses.

(function() {
  var Ember = this.Ember;
  if (typeof Ember === 'undefined' && typeof require !== 'undefined') {
    Ember = require('ember');
  }

  // here the module

})();

[marcoow]

I don't think Ember defines global define and require methods.

[marcoow]

Basically the AMD wrapper only exists because I wanted the library to show up in Ember's log message that prints all the registered libraries when the application starts. So it would probably make sense to simply change it to

//amd.start
(function(global) {
  var define = global.define;
  var require = global.require;
  var Ember = require('ember');
  Ember.libraries.register('Ember Simple Auth', '{{ VERSION }}'); //register-library is injected here 

//amd.end
  })(this);

which should have the same effect as if the wrapper didn't exist at all if I'm not missing sth.

[jbescoyez]

Here is a PR that fix the bug: #325

Not sure it is the best way to fix it. Feedback welcome!

[marcoow]

It looks like other libraries don't use a wrapper at all for the AMD distributions. Given that I think })(this); like I posted above makes more sense as that should be the same then as having no wrapper at all (at least I think that).

[jbescoyez]

Right. Updating the PR.

[jbescoyez]

Done #325 !

[dogawaf]

Using })(this); without changing anything else is the simpliest (universal) solution.

[marcoow]

should be fixed by #325