Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible breaking changes with Beta.11 and injectAsync? #126

Closed
chron0 opened this issue Aug 5, 2016 · 14 comments
Closed

Possible breaking changes with Beta.11 and injectAsync? #126

chron0 opened this issue Aug 5, 2016 · 14 comments

Comments

@chron0
Copy link
Contributor

chron0 commented Aug 5, 2016

I've bumped my project (with tests based on clicker) to Ionic@Beta.11 - failing here:

05 08 2016 20:15:07.917:ERROR [framework.browserify]: bundle error
05 08 2016 20:15:07.920:ERROR [framework.browserify]: TypeScript error: tests/diExports.ts(3,10): Error TS2305: Module '"/home/chrono/src/governess/governess-app/node_modules/@angular/core/testing"' has no exported member 'injectAsync'.
05 08 2016 20:15:08.144:INFO [karma]: Karma v1.1.0 server started at http://localhost:9876/
05 08 2016 20:15:08.144:INFO [launcher]: Launching browser PhantomJS with unlimited concurrency
05 08 2016 20:15:08.151:INFO [launcher]: Starting browser PhantomJS
05 08 2016 20:15:09.463:INFO [PhantomJS 2.1.1 (Linux 0.0.0)]: Connected on socket /#gKmIrdoZOy1QrbOEAAAA with id 36309521
05 08 2016 20:15:39.471:WARN [PhantomJS 2.1.1 (Linux 0.0.0)]: Disconnected (1 times), because no message in 30000 ms.
PhantomJS 2.1.1 (Linux 0.0.0) ERROR
  Disconnected, because no message in 30000 ms.

Current angular2 docs say that there are no new docs yet but there seem to have been changes.

@dorontal
Copy link
Contributor

dorontal commented Aug 6, 2016

I recall seeing this before in my own project, which has been linking with beta-11 before it was officially named so (i.e. it's been linking with the current ionic2 master for a while and this change happened a while ago). The fix is easy. There is no more injectAsync(), but inject() is there and async() is there and the way to get the old behavior is to import those two functions instead of the function injectAsync()

import {
    async,
    inject
} from '@angular/core/testing';

and then call

async(inject(.....arguments as given to injectAsync() before this change...)  )

instead of injectAsync(...)

I'm away this weekend and cannot create a pull request for this until Monday, but can do so then if nobody beats me to it.

---- On Fri, 05 Aug 2016 16:26:23 -0400 chrono notifications@github.com wrote ----

I've bumped my project (with tests based on clicker) to Ionic@Beta.11 - failing here:
05 08 2016 20:15:07.917:ERROR [framework.browserify]: bundle error05 08 2016 20:15:07.920:ERROR [framework.browserify]: TypeScript error: tests/diExports.ts(3,10): Error TS2305: Module '"/home/chrono/src/governess/governess-app/node_modules/@angular/core/testing"' has no exported member 'injectAsync'.05 08 2016 20:15:08.144:INFO [karma]: Karma v1.1.0 server started at http://localhost:9876/05 08 2016 20:15:08.144:INFO [launcher]: Launching browser PhantomJS with unlimited concurrency05 08 2016 20:15:08.151:INFO [launcher]: Starting browser PhantomJS05 08 2016 20:15:09.463:INFO [PhantomJS 2.1.1 (Linux 0.0.0)]: Connected on socket /#gKmIrdoZOy1QrbOEAAAA with id 3630952105 08 2016 20:15:39.471:WARN [PhantomJS 2.1.1 (Linux 0.0.0)]: Disconnected (1 times), because no message in 30000 ms.PhantomJS 2.1.1 (Linux 0.0.0) ERROR Disconnected, because no message in 30000 ms. Current angular2 docs say that there are no new docs yet but there seem to have been changes.

You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@chron0
Copy link
Contributor Author

chron0 commented Aug 6, 2016

@dorontal: Yes, awesome, that fixes the bundle error. I've pasted the specific changes to test/diExports.ts for reference, so that you you or @lathonez can pick it up directly.

--- ../../clicker/test/diExports.ts 2016-06-29 18:15:12.308410517 +0000
+++ tests/diExports.ts  2016-08-06 07:47:24.941769412 +0000
-import { injectAsync }                                from '@angular/core/testing';
+import { inject, async }                              from '@angular/core/testing';

-export let injectAsyncWrapper: Function = ((callback) => injectAsync([TestComponentBuilder], callback));
+export let injectAsyncWrapper: Function = ((callback) => async(inject([TestComponentBuilder], callback)));

However, I guess now I'm just stuck with an issue similar to #125, where the only major difference introduced here was the move from karma 0.13.22 to 1.1.0

> gulp --gulpfile tests/gulpfile.ts --cwd ./ unit-test

[08:08:21] Requiring external module ts-node/register
[08:08:24] Using gulpfile /home/chrono/src/governess/governess-app/tests/gulpfile.ts
[08:08:24] Starting 'unit-test'...
[08:08:24] Starting 'clean'...
[08:08:24] Finished 'clean' after 19 ms
[08:08:24] Starting 'lint'...
[08:08:24] Starting 'html'...
[08:08:26] Finished 'html' after 1.31 s
[08:08:26] Finished 'lint' after 1.88 s
[08:08:26] Starting 'karma'...
decorator { '0': [Function], '1': 0, '2': true, '3': undefined }

START:
06 08 2016 08:08:43.821:INFO [framework.browserify]: bundle built
06 08 2016 08:08:43.864:INFO [karma]: Karma v1.1.0 server started at http://localhost:9876/
06 08 2016 08:08:43.864:INFO [launcher]: Launching browser PhantomJS with unlimited concurrency
06 08 2016 08:08:43.873:INFO [launcher]: Starting browser PhantomJS
06 08 2016 08:08:44.151:INFO [PhantomJS 2.1.1 (Linux 0.0.0)]: Connected on socket /#SxcwQBKZRweymWqjAAAA with id 29330519
06 08 2016 08:09:14.158:WARN [PhantomJS 2.1.1 (Linux 0.0.0)]: Disconnected (1 times), because no message in 30000 ms.
PhantomJS 2.1.1 (Linux 0.0.0) ERROR
  Disconnected, because no message in 30000 ms.

Finished in 30.008 secs / 0 secs

[08:09:14] 'karma' errored after 48 s
[08:09:14] Error: 1
    at formatError (/home/chrono/src/governess/governess-app/node_modules/gulp/bin/gulp.js:169:10)
    at Gulp.<anonymous> (/home/chrono/src/governess/governess-app/node_modules/gulp/bin/gulp.js:195:15)
    at emitOne (events.js:82:20)
    at Gulp.emit (events.js:169:7)
    at Gulp.Orchestrator._emitTaskDone (/home/chrono/src/governess/governess-app/node_modules/gulp/node_modules/orchestrator/index.js:264:8)
    at /home/chrono/src/governess/governess-app/node_modules/gulp/node_modules/orchestrator/index.js:275:23
    at finish (/home/chrono/src/governess/governess-app/node_modules/gulp/node_modules/orchestrator/lib/runTask.js:21:8)
    at cb (/home/chrono/src/governess/governess-app/node_modules/gulp/node_modules/orchestrator/lib/runTask.js:29:3)
    at removeAllListeners (/home/chrono/src/governess/governess-app/node_modules/karma/lib/server.js:379:7)
    at Server.<anonymous> (/home/chrono/src/governess/governess-app/node_modules/karma/lib/server.js:390:9)
    at Server.g (events.js:260:16)
    at emitNone (events.js:72:20)
    at Server.emit (events.js:166:7)
    at emitCloseNT (net.js:1519:8)
    at nextTickCallbackWith1Arg (node.js:431:9)
    at process._tickCallback (node.js:353:17)
[08:09:14] 'unit-test' errored after 50 s
[08:09:14] Error in plugin 'run-sequence(karma)'
Message:
    karma callback

@chron0
Copy link
Contributor Author

chron0 commented Aug 6, 2016

I could solve this issue, by bootstrapping a fresh sidemenu ionic project and compared changes. Most notably were some changes in ionic's gulpfile and package.json:

diff --git a/governess-app/gulpfile.js b/governess-app/gulpfile.js
index 36a03bb..5f8a1da 100644
--- a/governess-app/gulpfile.js
+++ b/governess-app/gulpfile.js
@@ -32,6 +32,7 @@ var buildSass = require('ionic-gulp-sass-build');
 var copyHTML = require('ionic-gulp-html-copy');
 var copyFonts = require('ionic-gulp-fonts-copy');
 var copyScripts = require('ionic-gulp-scripts-copy');
+var tslint = require('ionic-gulp-tslint');

 var isRelease = argv.indexOf('--release') > -1;

@@ -41,12 +42,7 @@ gulp.task('watch', ['clean'], function(done){
     function(){
       gulpWatch('app/**/*.scss', function(){ gulp.start('sass'); });
       gulpWatch('app/**/*.html', function(){ gulp.start('html'); });
-      buildBrowserify(
-        {
-          src: ['./app/app.ts', './typings/index.d.ts'],
-          watch: true
-        }
-      ).on('end', done);
+      buildBrowserify({ watch: true }).on('end', done);
     }
   );
 });
@@ -76,3 +72,4 @@ gulp.task('scripts', copyScripts);
 gulp.task('clean', function(){
   return del('www/build');
 });
+gulp.task('lint', tslint);
diff --git a/governess-app/package.json b/governess-app/package.json
index 18023fe..a58d20e 100644
--- a/governess-app/package.json
+++ b/governess-app/package.json
@@ -56,6 +56,7 @@
     "ionic-gulp-html-copy": "1.0.0",
     "ionic-gulp-sass-build": "1.0.0",
     "ionic-gulp-scripts-copy": "2.0.1",
+    "ionic-gulp-tslint": "^1.0.0",
     "isparta": "4.0.0",
     "jasmine-core": "2.4.1",
     "jasmine-spec-reporter": "2.5.0",
@@ -73,6 +74,7 @@
     "tsify": "0.16.0",
     "tslint": "3.12.1",
     "tslint-eslint-rules": "1.3.0",
+    "tslint-ionic-rules": "^0.0.3",
     "typings": "1.3.1"
   },
   "cordovaPlugins": [

Maybe it will solve #125 too.

@lathonez
Copy link
Owner

lathonez commented Aug 8, 2016

@chron0 @dorontal thanks both for the input on this one. I'll update to beta.11 and incorporate your changes ASAP.

@lathonez
Copy link
Owner

lathonez commented Aug 9, 2016

Upgraded and encountered the same error as you guys:

START:
09 08 2016 13:48:25.902:ERROR [framework.browserify]: bundle error
09 08 2016 13:48:25.903:ERROR [framework.browserify]: TypeScript error: test/diExports.ts(3,10): Error TS2305: Module '"/home/lathonez/code/clicker/node_modules/@angular/core/testing"' has no exported member 'injectAsync'.

@chron0
Copy link
Contributor Author

chron0 commented Aug 9, 2016

Did you already make the changes noted above?

--- ../../clicker/test/diExports.ts 2016-06-29 18:15:12.308410517 +0000
+++ tests/diExports.ts  2016-08-06 07:47:24.941769412 +0000
-import { injectAsync }                                from '@angular/core/testing';
+import { inject, async }                              from '@angular/core/testing';

-export let injectAsyncWrapper: Function = ((callback) => injectAsync([TestComponentBuilder], callback));
+export let injectAsyncWrapper: Function = ((callback) => async(inject([TestComponentBuilder], callback)));

IIRC I had to delete & rebuild my whole node_modules as well...

@lathonez
Copy link
Owner

lathonez commented Aug 9, 2016

Yeah that's all good it's the RC4 form changes I'm struggling with atm.

@lathonez
Copy link
Owner

lathonez commented Aug 9, 2016

http://stackoverflow.com/questions/38316559/angular-2-0-0-rc-3-new-forms-e2e-specs-cant-bind-to-formgroup-cant-bind-t

No help there but a good reference point. Been digging for hours. Comparing differences between using formGroup in the app works (as expected) and ngFormGroup does not work (as expected). The idea being that when I find out why one works and the other doesn't we can try to figure out what's missing regarding formGroup in the test suite.

    TemplateParseVisitor.prototype._createElementPropertyAsts = function (elementName, props, directives) {
        var _this = this;
        var boundElementProps = [];
        var boundDirectivePropsIndex = new Map();
        directives.forEach(function (directive) {
            directive.inputs.forEach(function (prop) {
                boundDirectivePropsIndex.set(prop.templateName, prop);
            });
        });
        props.forEach(function (prop) {
            if (!prop.isLiteral && lang_1.isBlank(boundDirectivePropsIndex.get(prop.name))) {
                boundElementProps.push(_this._createElementPropertyAst(elementName, prop.name, prop.expression, prop.sourceSpan));
            }
        });
        return boundElementProps;
    };

The line that's blowing up is the _this._createElementPropertyAst, for the working case of formGroup, that function is never called as boundDirectivePropsIndex.get returns true, so lang_1.isBlank (basically a presence checker), returns false.

The reason it returns true for formGroup is that we pass directives in, and the boundDirectivePropsIndex variable gets sets to a large DirectiveAst object that seems to be a representation of the ngForm directive.

TL;dr - we're passing an ngForm directive into _createElementPropertyAsts for formGroup

TODO:

1 - what are we passing for ngFormGroup (if anything)
2 - compare to formGroup in the karma debug

@lathonez
Copy link
Owner

lathonez commented Aug 9, 2016

The above issue

    Failed: Template parse errors:
    Can't bind to 'formGroup' since it isn't a known native property ("<form [ERROR ->][formGroup]="form" (submit)="newClicker(form.value)">
      <ion-row>
        <ion-col width-80>
    "): ClickerForm@0:6

Is solved by adding REACTIVE_FORM_DIRECTIVES to the component in question.

@lathonez
Copy link
Owner

lathonez commented Aug 9, 2016

Now banging my head against:

  ClickerForm
    ✖ initialises
      PhantomJS 2.1.1 (Linux 0.0.0)
    Failed: EXCEPTION: Error in ./TextInput class TextInput - inline template:1:25
    ORIGINAL EXCEPTION: No value accessor for ''
    ERROR CONTEXT:
    [object Object]

For which this looks relevant:

ionic-team/ionic-framework#7125

@lathonez
Copy link
Owner

lathonez commented Aug 9, 2016

Nope, already got those fixes. The basic point for all of these is that bootstrapping the app as follows:

ionicBootstrap(ClickerApp, [
  disableDeprecatedForms(),
  provideForms(),
  Clickers,
  provide('Storage', {useClass: Storage})]
);

Sorts out the forms and little else (if anything?) is needed for the app to work using ionic serve

The problem we've got is getting the same bootstrapping through testing, which is why we're needing to jump through these hoops.

@lathonez
Copy link
Owner

lathonez commented Aug 9, 2016

Setting identical providers in diExports sorted it (as above basically). Pushed the commit upgrading to beta.11, RC4 and new forms.

@lathonez lathonez closed this as completed Aug 9, 2016
@lathonez
Copy link
Owner

lathonez commented Aug 9, 2016

01530a7

@chron0
Copy link
Contributor Author

chron0 commented Aug 9, 2016

Awesome, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants