sglanzer-deprecated/ember-cli-blanket

using ember-cli-yadda with ember-cli-blanket 'Error: Could not find module yadda'

Closed this issue · 20 comments

We have installed the latest versions of ember-cli-yadda and ember-cli-blanket, with ember-cli 1.13.8. When running /tests without blanket coverage the BDD tests run fine. When running /tests?coverage to enable blanket coverage, the BDD tests fail with:

Died on test #1     at Object.TestLoader.moduleLoadFailure (http://localhost:4200/assets/test-support.js:5854:11)
    at Object.TestLoader.require (http://localhost:4200/assets/test-loader.js:31:14)
    at Object.TestLoader.loadModules (http://localhost:4200/assets/test-loader.js:21:18)
    at Function.TestLoader.load (http://localhost:4200/assets/test-loader.js:40:24)
    at http://localhost:4200/assets/test-support.js:5863:16: Could not find module yadda@ 1 ms
Source:     
Error: Could not find module yadda
    at missingModule (http://localhost:4200/assets/vendor.js:125:11)
    at requireModule (http://localhost:4200/assets/vendor.js:135:17)
    at require (http://localhost:4200/assets/blanket-loader.js:128:16)
    at http://localhost:4200/assets/sf-todo.js:1201:23
    at mod.state (http://localhost:4200/assets/vendor.js:150:29)
    at tryFinally (http://localhost:4200/assets/vendor.js:30:14)
    at requireModule (http://localhost:4200/assets/vendor.js:148:5)
    at require (http://localhost:4200/assets/blanket-loader.js:128:16)
    at oldLoader (http://localhost:4200/assets/blanket-loader.js:84:13)
    at Object._blanket.extend.utils.processFile (http://localhost:4200/assets/test-support.js:12100:17)

Has anyone else seen this? I have also opened kaliber5/ember-cli-yadda#15 that has a bit more info.

@sglanzer definitely need to push for a core hook in loader.js to get out of this business.

I'm down and out for the next two weeks but happy to provide feedback on a generalized loader.js hook for intercepting 'require' and 'import' calls and returning resolved modules

cc: @stefanpenner

@sglanzer definitely need to push for a core hook in loader.js to get out of this business.

I'm open to this, not quite sure what sort of hook is needed. If someone can give me a good TL;DR.

@stefanpenner

My rapid response is:

  1. loader.js provides a hook that we would call to register ourselves as a moduleAnnotator
  2. as loader.js responds to require or 'importcalls it passes the resolved code to themoduleAnnotator` (if present) and returns the 'annotated' code

In our case we would do something like

requirejs.annotator = function(name, resolvedModule) {
   // our logic to determine if it should be covered
  let result = resolvedModule;
   if (shouldCover) {
    result = blanket.instrument({
      inputFile: resolvedModule,
      inputFileName: name
    });
  }
  return result;
}

Annotator should be called once per newly 'loaded' module (import or require)

we could unwind a lot of hackery with this.

@sglanzer - any other requirements you can think of?

we're currently hooking require here: https://github.com/sglanzer/ember-cli-blanket/blob/master/vendor/blanket-require.js#L74

resolvedModule

is resolvedModule the callback or .. ?

yeah, hmmm, it used to be the entry in requirejs.entries[moduleName] but now that's a Module

we probably don't want to leak that abstraction. I think it should be the exports of the resolved module.

I should probably put together a rough prototype to make sure we have the api correct

I think it should be the exports of the resolved module.

that makes sense.

I'm picking at it - trying to put together a flow that works.

In the old world, we would have blanket instrument the code by wrapping the exports in a new define statement (which when evaled by blanket) would seed the requirejs cache.

Blanket does this instrumentation and calls a callback. So I need to unwind it a bit to make it synchronous (allowing the above proposal to work) or figure something else out.

will chip away at it this weekend

maybe something like:

require.addAnnotator('banket', function(exports) {
  return transform(exports);
});

yes, I think that will do it, trying to find right place to call 'annotator' in loader.js to try it out

so here's the problem - by the time exports are reified they are already 'evaled' afaict. I need to do a source level transform. I could be wrong - will investigate and get back to you

@jschilli ya, thats why i was thinking the callback, which can be munged and re-attached. Just doing it at define time would be clever IMHO

If you can point me to a line in loader where you think that would go I will verify

-jeff

On Dec 18, 2015, at 5:18 PM, Stefan Penner notifications@github.com wrote:

@jschilli ya, thats why i was thinking the callback, which can be munged and re-attached.


Reply to this email directly or view it on GitHub.

Awesome. Will advise

On Dec 18, 2015, at 5:20 PM, Stefan Penner notifications@github.com wrote:

https://github.com/ember-cli/loader.js/blob/master/loader.js#L145


Reply to this email directly or view it on GitHub.

probably needs to be deeper than that after some investigation e.g. https://github.com/ember-cli/loader.js/blob/master/loader.js#L113

hooking into define works for top level objects and could work by navigating dependencies in the transformer/annotator.

Hooking into findModule or reify would provide direct access to all required/imported modules as they are resolved.

I think the primary challenge (and not saying that I'm discounting it yet) with doing so @ define only is that in the case of blanket it's ultimately evaling the annotated code and hence resolving not just the dependencies of the module, but the children dependencies as well.

Long day here - will mark up a sample project and determine the right place (or places) to hook

Thanks for the assist and push

One your only annotate modules who's module name matched something. Allowing those from deps to be filtered out

Hey - just joining the conversation - we have exclusion loaders if that's what you're referring to (not sure if I'm reading the last comment correctly).

Agreed with @jschilli - to do this in a non-hack manner we need access to the layer below define - sounds like that's findModule or reify.

What we used to do was hook into the define lookups, rip the dependencies array apart for each module and instrument (annotate) all of those. This only extends to one level of dependencies, it should go deeper - but we also want to make sure that when we go deeper we exclude anything that's not from the target project (which would solve another one of our long running issues).

I assume every dependency in the application has to pass through the loader at some point - is that point findModule?

Sorry if I'm rehashing anything that has already been discussed.

I have been hacking at this for more than a few hours and although I’ve gotten some paths to work, others remain uncovered.

I see this as having two primary challenges:

  1. where to hook loader.js to ensure everything is covered (the answer seems to be findModule although the exports method of Loader.js is another prime candidate)
  2. what is the interface and data flow between ember-cli-blanket, loader.js and blanket itself

The biggest challenge is that blanket is written to be async by default (processFile, instrumentFile etc) are async - so having a transformer that influences what is returned by loader will demand a synchronous approach

I’ve just started poking around in a synchronous refactor of blanket but don’t have it working yet.

Time is hard to come by ATM and I’m headed out on vacation Tuesday. Will update here if I have any breakthroughs.

Ok, i put together a synchronous implementation of blanket.instrument and am able to return the instrumented callback to loader.js at the point the module is finalized.

The mod to loader.js (hacked and not optimized) is:

  Module.prototype.exports = function(reifiedDeps) {
    if (this.finalized) {
      return this.module.exports;
    } else {
        if (requirejs.annotator) {
            this.callback = requirejs.annotator(this.name, this.deps, this.callback);
        }
      var result = this.callback.apply(this, reifiedDeps);
      if (!(this.hasExportsAsDep && result === undefined)) {
        this.module.exports = result;
      }
      this.makeDefaultExport();
      this.finalized = true;
    //   console.log('finalizing: ', + this.name);
      return this.module.exports;
    }
  }

We're not using deps in the version that I have working so I would expect the final api to be

this.callback = requirejs.annotator(this.name, this.callback);

AFAICT this logic will only be called as the module is finalized so the annotator should be called at most once for each loaded module.

In order for us to make this work (ember-cli-blanket) we'll need to get an approved PR up on loader.js and a change to blanket.js to support an instrumentSync call

I'll work on a proforma PR for both

Thanks Jeff, awesome work!

On Sun, Dec 20, 2015 at 7:29 PM, Jeff Schilling notifications@github.com
wrote:

Ok, i put together a synchronous implementation of blanket.instrument and
am able to return the instrumented callback to loader.js at the point the
module is finalized.

The mod to loader.js (hacked and not optimized) is:

Module.prototype.exports = function(reifiedDeps) {
if (this.finalized) {
return this.module.exports;
} else {
if (requirejs.annotator) {
this.callback = requirejs.annotator(this.name, this.deps, this.callback);
}
var result = this.callback.apply(this, reifiedDeps);
if (!(this.hasExportsAsDep && result === undefined)) {
this.module.exports = result;
}
this.makeDefaultExport();
this.finalized = true;
// console.log('finalizing: ', + this.name);
return this.module.exports;
}
}

We're not using deps in the version that I have working so I would expect
the final api to be

this.callback = requirejs.annotator(this.name, this.callback);

AFAICT this logic will only be called as the module is finalized so the
annotator should be called at most once for each loaded module.

In order for us to make this work (ember-cli-blanket) we'll need to get an
approved PR up on loader.js and a change to blanket.js to support an
instrumentSync call

I'll work on a proforma PR for both


Reply to this email directly or view it on GitHub
#103 (comment)
.