computmaxer / karma-jspm

Other
74 stars 50 forks source link

CP-727 Fix compatibility with SystemJS 0.17+ #61

Closed matoilic closed 9 years ago

matoilic commented 9 years ago

SystemJS 0.17+ requires the baseURL to be configured first and doesn't allow to change it once set. Also, es6-module-loader is not needed anymore. It has been replaced with systemjs-polyfills which are a stripped down version of es6-module-loader.

maxwellpeterson-wf commented 9 years ago

Thanks for the PR! +1

@evanweible-wf @trentgrover-wf

trentgrover-wf commented 9 years ago

+1

trentgrover-wf commented 9 years ago

@jayudey-wf ready for merge

evanweible-wf commented 9 years ago

+1

jayudey-wf commented 9 years ago

QA Resource Approval: +10

Merging into master.

martinmicunda commented 9 years ago

Is there plan to create new release and push it to npm?

scott-coates commented 9 years ago

Yes, please publish a new version on npm please. Thanks.

martinmicunda commented 9 years ago

@matoilic I tried your changes but I can't run any tests written in ES6 (I can run tests that are written in ES5)

describe('Employee', function(){
    const pp = 'test'; ----> this ES6 code cause that test doesn't run
    it('should run this test', function() {
        expect(true).toBe(true);
    });
});

output

PhantomJS 1.9.8 (Mac OS X 0.0.0): Executed 0 of 0 ERROR (0.001 secs / 0 secs)
martinmicunda commented 9 years ago

@matoilic sorry got that running with Chrome .. It seems ES6 doesn't work with PhantomJS

scott-coates commented 9 years ago

In my config.js, I've set the baseURL to "/". I think this is recommended.

So when init.js configures the expandedFiles property, my config.js file will be called, overwrriting the baseURL set here in karma-jspm. So if baseURL is overwritten, won't we be in the same situation?

Am I missing something? Thanks!

martinmicunda commented 9 years ago

@scoarescoare I got that run but with complications against master. My baseRL is set to "./" then I had to add this lines to my karma.conf.js file

        proxies: {
            '/jspm_packages/': '/base/jspm_packages/'
        },

then change this lines in init.js line 105 --> see bug #63

client.jspm.expandedFiles = flatten(jspm.loadFiles.map(function(file){
    files.push(createServedPattern(basePath + "/" + (file.pattern || file)));
    return expandGlob(file, basePath).map(function(path) {
      return '/base/' + path;
    });
  }));
scott-coates commented 9 years ago

I was in a rush to just get the tests to run for now, so I just did this:

// hack to get around karma-jspm - https://github.com/Workiva/karma-jspm/pull/61
const basePath = /\/base\//.test(System.baseURL) ? System.baseURL : "/";

System.config({
  "baseURL": basePath,
  ...
});

The idea being that if I was loading the environment from karma, I can assume that karma-jspm will have already set baseURL, so this is just a hack to not actually overwrite karma-jspm's config setting.

dmackerman commented 9 years ago

@martinmicunda ES6 works with PhantomJS if you include a bind() polyfill. I'm going to try your hack @scoarescoare.

omerts commented 9 years ago

+1 for releasing to npm

maxwellpeterson-wf commented 9 years ago

@martinmicunda @scoarescoare @mrydengren @omerts karma-jspm@2.0.0-beta.1 has been released and published to npm.

scott-coates commented 9 years ago

Thanks!