Add a nativescript zone implementation #440
Conversation
Avoid patching Promise twice by checking if the promise-like returned by fetch() is already an instance of ZoneAwarePromise.
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
|
I signed it! |
|
CLAs look good, thanks! |
1 similar comment
|
CLAs look good, thanks! |
| @@ -0,0 +1,842 @@ | |||
| var Zone$1 = (function (global) { | |||
There was a problem hiding this comment.
Please Remove the changes to the dist folder. We update the dist folder on releases only.
| @@ -169,3 +174,31 @@ gulp.task('test/node', ['compile'], function(cb) { | |||
| jrunner.addSpecFiles(specFiles); | |||
| jrunner.execute(); | |||
| }); | |||
|
|
|||
| gulp.task('test/nativescript', ['compile'], function(cb) { | |||
There was a problem hiding this comment.
Please also update the package.json file so that native scripts can be run using npm run test-nativescript command.
| @@ -1148,7 +1148,7 @@ const Zone: ZoneType = (function(global: any) { | |||
| const fetchPromise = global['fetch'](); | |||
| // ignore output to prevent error; | |||
| fetchPromise.then(() => null, () => null); | |||
| if (fetchPromise.constructor != NativePromise) { | |||
| if (fetchPromise.constructor != NativePromise && fetchPromise.constructor != ZoneAwarePromise) { | |||
There was a problem hiding this comment.
Can you break this change into a separate commit. I don't think it has anything to do with NativeScript
There was a problem hiding this comment.
Already done. The PR contains both commits.
|
|
||
| // List all tests here: | ||
| import './common_tests'; | ||
| import './nativescript_tests'; |
There was a problem hiding this comment.
nativescript_tests.ts seems to be an empty file. Was that intended?
There was a problem hiding this comment.
I thought it would look consistent with the node_tests.ts setup, but I can delete it. Your call.
Based on the zone-node entrypoint with the timers module patching removed. Also add rudimentary support for running NativeScript tests under node.js (to avoid setting up device emulators).
hdeshev
force-pushed
the
zone-nativescript
branch
from
8214dea to
e6c01d8
Compare
The nativescript-angular mobile renderer was using the zone-node.js bundle until 0.6.18+ added patching for the
timersmodule andfetchpromise-likes. Those are incompatible with NativeScript and cause runtime errors. The best solution to support zone.js in NativeScript in a way that minimizes future breakage seems to involve creating a separate bundle.I based this bundle on the node one, just removing the unsupported patches. I also added a rudimentary test task that runs under node (to avoid spinning up device emulators and complicate build infrastructure).
nativescript-angularvendors thezone-nativescript.jsbundle that this PR adds, but it would be great if we get rid of that in the future.This PR takes care of #437 and the
fetch()change is somewhat related to #439.