dfreeman / ember-cli-node-assets

Include stylesheets, images and other assets in your Ember app from node modules
MIT License
62 stars 6 forks source link

Trouble importing library with UMD wrapper #27

Open cah-stephenhartshorne opened 7 years ago

cah-stephenhartshorne commented 7 years ago

So I am trying to import a module and am seeing the following error: this.callback.apply is not a function

My ember-cli-config looks like:

let config = {};

config.nodeAssets = {
  pact: {
    vendor: {
      include: ['pact-web.js', 'pact-web.js.map'],
      srcDir: 'dist',
    }
  }
};

let app = new EmberApp(defaults, config);

app.import('vendor/pact/pact-web.js', {
  using: [ { transformation: 'amd', as: 'Pact' } ]
});

When I was debugging, I saw that the module is doing this check when it gets loaded:

if(typeof exports === 'object' && typeof module === 'object')
        module.exports = factory();
    else if(typeof define === 'function' && define.amd)
        define("Pact", [], factory);
    else if(typeof exports === 'object')
        exports["Pact"] = factory();
    else
        root["Pact"] = factory();

and executes define("Pact", [], factory);

I also found that when var result = this.callback.apply(this, this.reified); is called inside loader.js (AKA the line that gives me the error), callback in this case is "Pact".

When I change the definition from define("Pact", [], factory); to define([], factory);, the module loads correctly and I am able to import it just fine.

Is there another configuration I'm missing in my ember-cli-build? Or is this a problem with pact.js?

(For reference, pact js is here: https://github.com/pact-foundation/pact-js)

dfreeman commented 7 years ago

Hi @cah-stephenhartshorne—funnily enough, I actually just ran into this with pact a couple days ago. It turns out pact-web is kind of a bad UMD citizen; it's trying to define a named AMD module rather than an anonymous one, and as far as I can tell the amd transformation in ember-cli doesn't handle that.

One workaround would be to skip the transformation in the import and run ember g vendor-shim Pact, then import the resulting file.

Changing topics slightly, I'd love to know more about what you're aiming to do with Pact, if you're willing to share. I'm neck deep in work on an ember-cli + pact (+ mirage) integration right now that I'm aiming to have cleaned up and released in time for the talk I optimistically agreed to give on it at Boston Ember in a couple weeks 😄

cah-andrew-fitzgerald commented 7 years ago

That's really funny that you're also doing pact stuff. We've been using pact for quite a while for testing interactions between our java microservices, and wanted to start testing the waters of pact-js/ember.

We've got a legacy coldfusion app that we're rewriting portions of, and we were hoping to use Pact to help us TDD the server portion of it. We've already the got the UI built out and running with mirage, and were trying to wire up pact so that we could generate a pact file before we start building out our JVM endpoints.

Gonna page @mefellows (one of the main pact-js guys), I was just asking him about this in the pact gitter channel

mefellows commented 7 years ago

It turns out pact-web is kind of a bad UMD citizen; it's trying to define a named AMD module rather than an anonymous one, and as far as I can tell the amd transformation in ember-cli doesn't handle that.

I'll raise an issue for this over at the Pact JS repository, if we're not being good citizens then we need to get in line!

mefellows commented 7 years ago

FYI have an issue created at pact-js repo (https://github.com/pact-foundation/pact-js/issues/98) and now understand the problem a bit better. We'll release a new major version in the next few weeks along with the upgrade to TypeScript. It sounds like there is a workaround for now, if not I can do it in mainline and bump a major version there (but i'll be bumping it again fairly soon!).

dfreeman commented 7 years ago

@cah-andrewfitzgerald can weigh in if he disagrees, but I think the workaround should be fine for now until your next major version bump. Thanks @mefellows!

cah-andrew-fitzgerald commented 7 years ago

Yep, sounds like it (but I'm gonna need you to keep me up to date on your pact-ember experiments)