-
Notifications
You must be signed in to change notification settings - Fork 12k
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
broccoli bump and tools update #206
Conversation
…lar Beta6. Bumping broccoli version as well as updating tool compilations and maps.
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
Does this still happen with the latest master? It should be fixed there, we're late with releasing a new version though. |
It does, I was running off of 6db91e7 and was still getting the issue against beta6. |
CLAs look good, thanks! |
Ok, can you show me reproduction steps for these errors please? I haven't seen them yet so I don't know how your PR fixes them. |
Working on replicating it again. I wiped and did a clean install again against master and was not able to replicate. I noticed some odd behavior before when I went from beta3 to beta5 as well and had to wipe my node_modules directory. Possibly this was an artifact of moving from beta5 to beta6 and was only "resolved" when I wiped my node module directory in testing the angular-cli updates. Investigating now. |
Roger. We had loads of trouble with the upgrade last week so it scares me to think it might still be broken, and we just don't know. Needless to say, thanks a bunch for going ahead and fixing it. The upgrade was a hard one. |
I did some testing and it appears that I got my node_modules directories in a weird state in the process of bumping angular and angular-cli. After wiping both it looks like Master does work (was not working Friday of last week) which confirms what you were saying. However I did notice that the perf stats appeared to be a little better on new versions of broccoli. So might be a merge worth doing still. |
I think the brocolli setup is supposed to be similar to the one found in the angular repo, but I might be wrong. If that is the case, it's better to not deviate much from those versions. @IgorMinar? In either case, this PR as is changes more than (what I suppose to be) required for the version change, so in that sense there is a bit of an increased surface area for bugs. There is also a new warning about a missing file appearing on the travis tests. It's probably best to wait for Igors input on this before putting more work into it. |
The broccoli files are compiled version of the typescript files found in angular/angular. Did you compile the latest from over there? |
@hansl yes I did, pulled from the latest and compiled them and dropped them in. |
LGTM, I will commit this on your behalf. Thanks a lot for the update. |
Had to revert the use of |
Only IE11 and less needs it. See http://kangax.github.io/compat-table/es6/\#test-Reflect Fixes #206.
Only IE11 and less needs it. See http://kangax.github.io/compat-table/es6/\#test-Reflect Fixes #206.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
angular-cli throws multiple errors for Cannot read property with Angular2 Beta6. Bumping broccoli version as well as updating tool compilations and maps.