feat(http): add support for JSONP requests by caitp · Pull Request #2905 · angular/angular

caitp · 2015-07-06T20:24:53Z

Dart is broken, WIP

Closes #2818

caitp · 2015-07-06T20:27:41Z

I'm not going to manually fix clang-format errors

When you run gulp enforce-format, it should give you the script to run to automatically fix all clang complaints.

The script is what produced the bad sources, out just looks really really bad

caitp · 2015-07-08T14:50:32Z

anyone who knows anything about dart have an idea here? the JS code is happy

jeffbcross · 2015-07-09T00:16:30Z

@caitp The latest travis build #6453 looks like it's just choking on TypeScript errors. Can you point to a build where dart errors were apparent?

jeffbcross · 2015-07-09T00:29:50Z

Fixing this missing semicolon, and moving async.done to the next line will get rid of all ts2dart errors.

jeffbcross · 2015-07-09T01:11:36Z

I've started addressing some of the dart issues on my fork: https://github.com/jeffbcross/angular/tree/pr-2905

I'll get back into it tomorrow and will connect with you on the right approach to get this working in dart, which is still not completely clear to me. Requires a little more research into dart.

jeffbcross · 2015-07-09T15:41:10Z

Yeah, I think my fork is not the direction to go. We'll most likely just need to create a browser_jsonp.dart similar to how I did with browser_xhr, rather than use the DomAdapter facade like I was trying to do.

caitp · 2015-07-10T19:27:41Z

PTAL --- it's working locally for me on dart2js and production js, and passing all cjs tests

caitp · 2015-07-10T19:32:20Z

actually, a slight fixup was needed for cjs that broke the dart build --- it seems ts2dart doesn't handle "protected" properties

caitp · 2015-07-10T20:37:40Z

Just blocked on dart-archive/ts2dart#240 I guess

jeffbcross · 2015-07-10T22:48:59Z

I merged dart-archive/ts2dart#240, but will have to revisit Monday because there's a release process for ts2dart that I'm not familiar with.

jeffbcross · 2015-07-10T22:49:47Z

Though you could update the PR to use latest ts2dart package if you want to see if Travis goes green.

caitp · 2015-07-11T01:30:01Z

I'll add a commit to test that tonight just in case the patch wasn't enough

caitp · 2015-07-11T02:01:15Z

well that didn't work

caitp · 2015-07-11T03:22:11Z

I'm not sure why the benchmark uses !isJsObject({}) rather than just JitChangeDetection.isSupported()?

jeffbcross · 2015-07-13T21:14:00Z

Whoa, Travis is actually green! Is this ready for review, @caitp?

caitp · 2015-07-13T21:18:29Z

yes, by all means =)

jeffbcross · 2015-07-13T21:36:26Z

Why not TypeError? https://api.dartlang.org/apidocs/channels/be/dartdoc-viewer/dart:core.TypeError

Dart's TypeError class doesn't let me attach an arbitrary message. I guess Dart doesn't use it as a catch-all exception the way JS does

jeffbcross · 2015-07-13T21:59:52Z

I don't understand this expectation. It would pass under any circumstance because you're comparing two string literals.

yeah, the wording makes sense but in practice it doesn't really work. We do need some kind of UNREACHABLE() assertion though

jeffbcross · 2015-07-13T22:06:07Z

Left some minor comments, mostly around tests being written in a way where expectations could be skipped, or aren't valid,

I'll cut the ts2dart release this afternoon, and with the comments addressed, should be good to merge. I'll wait to give it green label until ts2dart is released.

caitp · 2015-07-13T22:20:17Z

yeah, will make the suggested changes

caitp · 2015-07-14T02:00:49Z

Those commits should address the review comments, still waiting on the ts2dart release so that I can pull that in here. I'll squash when checking in

jeffbcross · 2015-07-15T00:09:43Z

TS2Dart has been updated: 3810e4b

jeffbcross · 2015-07-15T01:11:38Z

LGTM! Merge when Travis goes green.

angular-automatic-lock-bot · 2019-09-06T18:22:43Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes label Jul 6, 2015

caitp reviewed Jul 6, 2015
View reviewed changes

tbosch added the state: WIP label Jul 7, 2015

tbosch assigned jeffbcross Jul 8, 2015

tbosch added the action: review The PR is still awaiting reviews from at least one requested reviewer label Jul 8, 2015

jeffbcross reviewed Jul 9, 2015
View reviewed changes

caitp force-pushed the jsonp branch from e42c1e2 to de010e4 Compare July 10, 2015 19:26

caitp force-pushed the jsonp branch from de010e4 to e979ec2 Compare July 10, 2015 19:29

caitp force-pushed the jsonp branch from e979ec2 to 962a771 Compare July 10, 2015 20:19

caitp force-pushed the jsonp branch from 962a771 to 7e5434a Compare July 11, 2015 01:55

caitp force-pushed the jsonp branch from 7e5434a to 38cc2b4 Compare July 11, 2015 02:28

jeffbcross reviewed Jul 13, 2015
View reviewed changes

caitp force-pushed the jsonp branch from 26108e0 to 491a7f4 Compare July 14, 2015 01:59

feat(http): add support for JSONP requests

752e084

caitp force-pushed the jsonp branch from 06c89f3 to 752e084 Compare July 15, 2015 00:53

jeffbcross added pr_state: LGTM action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer state: WIP labels Jul 15, 2015

caitp closed this in 81abc39 Jul 15, 2015

angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 6, 2019

Conversation

caitp commented Jul 6, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caitp commented Jul 8, 2015

Uh oh!

jeffbcross commented Jul 9, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeffbcross commented Jul 9, 2015

Uh oh!

jeffbcross commented Jul 9, 2015

Uh oh!

caitp commented Jul 10, 2015

Uh oh!

caitp commented Jul 10, 2015

Uh oh!

caitp commented Jul 10, 2015

Uh oh!

jeffbcross commented Jul 10, 2015

Uh oh!

jeffbcross commented Jul 10, 2015

Uh oh!

caitp commented Jul 11, 2015

Uh oh!

caitp commented Jul 11, 2015

Uh oh!

caitp commented Jul 11, 2015

Uh oh!

jeffbcross commented Jul 13, 2015

Uh oh!

caitp commented Jul 13, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeffbcross commented Jul 13, 2015

Uh oh!

caitp commented Jul 13, 2015

Uh oh!

caitp commented Jul 14, 2015

Uh oh!

jeffbcross commented Jul 15, 2015

Uh oh!

jeffbcross commented Jul 15, 2015

Uh oh!

angular-automatic-lock-bot Bot commented Sep 6, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants