kbaltrinic / http-backend-proxy

A proxy that allows configuring the ngMockE2E $httpBackend service from within protractor based tests.
MIT License
84 stars 14 forks source link

Not working with JSPM #27

Open JSlain opened 8 years ago

JSlain commented 8 years ago

I'm using JSPM for my front-end dependencies management. No modules are polluting the global namespace, including 'angular'.

Generally, when i use a third-party library that need angular in the global namespace, i can hack it in the JSPM config file, using a shim. I don't fully undertsand how http-backend-proxy works, but it would be easy for me if i could simply add something like... var angular = require('angular'); ... at the beginning of the generated module code. Is there a point of insertion for me to inject some custom code before the module get sent to the browser?

My current error is: Failed: unknown error: angular is not defined And i definitely don't wanna add angular directly in my page, since it'ld break the purpose of the E2E test. (angular IS NOT polluting the global namespace once truly deployed)

kbaltrinic commented 8 years ago

Sad to say, its been so long since I have been in this code, I am not sure I remember how it works either. :-) However I am pretty sure it would be easy to do what you are trying.

Every script that is sent to the browser passes through the wrapScriptWithinInjectorInvoke function. Line 72 is likely the cause of your error.

It should not be too hard to modify this method to test for a new key in the options hash (maybe options.setupScript) and if its not null, prepend its content to the script generated in wrapScriptWithinInjectorInvoke.

Would be happy to entertain a PR to that effect; it would need to include appropriate tests and documentation updates of course.

JSlain commented 8 years ago

Ok i'm working on it.

Tried a few things, but that won't do the trick.

In the JSPM world, dependencies are resolved at runtime, instead of including them manually with <script/> tags. The result i'ld like to have:

    <body ng-app="app">
        <script src="jspm_packages/system.js"></script>
        <script src="config.js"></script>

        <!-- Beginning of injected part -->
        <script>
          (function(){
            var el = document.querySelector("' + browser.rootEl + '");\
            System.import('angular').element(el).injector().invoke(["$httpBackend", function($httpBackend){ /* the user script */}]);'; 
          })();
        </script>
        <!-- End of injected part -->

        <script>
          System.import('src/main.js');
          System.import('angular-mocks');
        </script>

In fact, 2 things differs from the current behavior:

Do you think there's a way we could make the code configurable to allow us to obtain this result? It appears to me that this is so specific to JSPM... i should simply make a new plugin out of yours.

For sure, I see ways of modifying your plugin, making it modular concerning the final script being injected, but i think that would just add a smell; which i'm the only one being really using it.

What do you think?

kbaltrinic commented 8 years ago

As for making the code configurable to obtain the result you are looking for, you could need to introduce and option perhaps called 'angularRef' that would default to angular and which you could set to System.import('angular'). Then update the code generation to use that option any time it needs to refer to angular. That seems like a nice enhancement to me that doesn't smell at all.

JSlain commented 8 years ago

Yeah, good idea. I'll work on it.

I tried to start, but i some tests are failing, all related to regex. I also tried the build in travis, and realized you were running node 0.10 Is it a requirement, or it should still pass using current version of node?

If so, i'll also try to correct these tests.

JSlain commented 8 years ago

I'm having a hard time fixing those tests, 'cause i don't understand them.

For example, why do these 2 cases shouldn't have the same result?

{
    regex : /\/a\/path/,
    output: 'new RegExp("\\\\/a\\\\/path")',
    desc  : 'litterals with forward slashes'
}, {
    regex : new RegExp('/a/path'),
    output: 'new RegExp("/a/path")',
    desc  : 'constructors with forward slashes'
}
kbaltrinic commented 7 years ago

Sorry to be late in replying. The only reason we are using a older version of node it that its been that long since the code has been touched. As for the above code, I would need to do some manual testing to understand why I took that approach. Afraid I don't recall this far removed. It does seem odd and may possibly be wrong. I appreciate your effort and will try to put a little time into it later this week. Do you have a fork you are working off of that I can look at. May be able to contribute even.

kbaltrinic commented 7 years ago

So I had a chance to play with this and while you could if you wanted to rewrite the first test to

{
    regex : /\/a\/path/,
    output: 'new RegExp("/a/path")',
    desc  : 'litterals with forward slashes'
}

it would require (slightly) more complex code in the if(obj instanceof RegExp){ ... } block of the code under test in order to make that pass, while the existing code and test are valid. Indeed, whether you paste eval( 'new RegExp("\\\\/a\\\\/path")' ) or eval( 'new RegExp("/a/path")' ) into the Chrome console, both return /\/a\/path/.

At the root of it is that we are simply calling JSON.stringify on the regex's, in the one case /a/path and in the other case \/a\/path and then represent the resulting string as a string in JSON, requiring an additional escaping of the backslashes (a sort of a stringify^2 if you will).