build: reference zone.js directly from source instead of npm #33046

JiaLiPassion · 2019-10-09T04:38:57Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the current behavior?

Issue Number: #32482

What is the new behavior?

reference zone.js directly from source instead of npm

Does this PR introduce a breaking change?

Yes
No

alexeagle · 2019-10-09T22:08:33Z

tools/cjs-jasmine/index-tools.ts

@@ -9,23 +9,23 @@
 'use strict';

 const glob = require('glob');
-require('zone.js/dist/zone-node.js');
+import '@angular/zone.js/lib/node/rollup-main';


I don't think these imports should change - the package is zone.js not @angular/zone.js

something needs to make the mapping available to type-checker and runtime

Got it, so it should be import 'zone.js/lib/node/rollup-main' or you mean keep it like require('zone.js/dist/zone-node.js');? Because currently we only have the source not the built npm_package. So in this PR.

for packages/* (such as packages/core), I updated the reference to zone.js source code, just like this one, import '@angular/zone.js/lib/node/rollup-main';

for modules/*, I updated the reference to zone.js dist bazel rule.

for integrations/*, I added build zone.js npm_package first, and add zone.js built folder to package.json.

we should never import '@angular/zone.js/... - why is that needed? Bazel should understand a zone.js import

Got it, I have updated all @angular/zone.js to zone.js.

alexeagle · 2019-10-11T19:37:27Z

packages/core/test/render3/load_domino.ts

@@ -7,11 +7,10 @@
 */

 // Needed to run animation tests
-require('zone.js/dist/zone-node.js');
-
+import '@angular/zone.js/lib/node/rollup-main';


this doesn't look right. what do we need to fix so the 'zone.js/...' import works?

Do you mean import zone.js/lib/node/rollup-main? or import zone.js/dist/zone-node.js.

to support import zone.js/lib/node/rollup-main? We just need to update the path mapping in tsconfig and some other mapping, I will make a list.

to support import zone.js/dist/zone-node.js., we need to build //packages/zone.js:npm_package first and then update the `path mapping to "zone.js": "dist/bin/packages/zone.js/npm_package"

Ideally Angular would depend directly on the TS outputs of zone, not on the bundled rollup distribution, because the latter is a longer re-build. So ideally I think you want 1.

@alexeagle , thank you, and I have updated the reference to zone.js. Please review, thanks!

alexeagle · 2019-10-25T13:45:22Z

packages/zone.js/dist/BUILD.bazel

    outs = ["zone.js.d.ts"],
-    cmd = "find $(SRCS) -name \"zone.d.ts\" -exec cp {} $(@D)/zone.js.d.ts \;",
+    cmd = "cp $< $@",


alexeagle · 2019-10-25T13:47:55Z

integration/typings_test_ts35/package.json

@@ -0,0 +1,30 @@
+{


this file is surprising - do you mean to add it? bad merge maybe?

Yeah, thanks, I have deleted it.

mhevery · 2019-11-05T00:04:18Z

presubmit

mhevery · 2019-11-05T00:05:10Z

MERGE_ASSISTANCE: Global Approval

atscott · 2019-11-05T21:16:02Z

The expanding_rows benchmark test appears to be failing with error http://<server>/angular/packages/zone.js/dist/zone.test.min.js - Failed to load resource: the server responded with a status of 404 (Not Found) @mhevery @JiaLiPassion Can one of you take a look?

Close angular#32482

JiaLiPassion · 2019-11-05T23:53:27Z

@atscott, thanks and I just updated the PR, please review.

atscott · 2019-11-06T00:06:11Z

presubmit

Close #32482 PR Close #33046

…r#33046) Close angular#32482 PR Close angular#33046

…st.sh` In angular#33046, internal uses of `zone.js` were switched to reference it directly from source (built with Bazel) instead of npm. As a result, the necessary scripts were updated to build `zone.js` as necessary. However, the `integration/ngcc/debug-test.sh` script was missed (probably because it is not used on CI, but only locally as a helper for debugging the ngcc integration project). This commit updates the `debug-test.sh` script to also build `zone.js` as needed.

…st.sh` In angular#33046, internal uses of `zone.js` were switched to reference it directly from source (built with Bazel) instead of npm. As a result, the necessary scripts were updated to build `zone.js` as necessary. However, some `integration/**/debug-test.sh` scripts were missed (apparently because they are not used on CI, but only locally as helpers for debugging the integration projects). This commit updates the `scripts/build-packages-dist.sh` script to also build `zone.js`, so that other scripts (such as the various `debug-test.sh` scripts) can use it.

…st.sh` (#33733) In #33046, internal uses of `zone.js` were switched to reference it directly from source (built with Bazel) instead of npm. As a result, the necessary scripts were updated to build `zone.js` as necessary. However, some `integration/**/debug-test.sh` scripts were missed (apparently because they are not used on CI, but only locally as helpers for debugging the integration projects). This commit updates the `scripts/build-packages-dist.sh` script to also build `zone.js`, so that other scripts (such as the various `debug-test.sh` scripts) can use it. PR Close #33733

angular-automatic-lock-bot · 2019-12-07T16:04:06Z

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.}

JiaLiPassion requested review from a team as code owners October 9, 2019 04:38

googlebot added the cla: yes label Oct 9, 2019

JiaLiPassion requested a review from IgorMinar as a code owner October 9, 2019 09:34

JiaLiPassion force-pushed the zone-bazel-ref branch 4 times, most recently from 9e03a8d to e931f22 Compare October 9, 2019 14:07

JiaLiPassion changed the title ~~WIP: reference zone.js directly from source instead of npm~~ build: reference zone.js directly from source instead of npm Oct 9, 2019

JiaLiPassion added the area: zones label Oct 9, 2019

ngbot bot added this to the needsTriage milestone Oct 9, 2019

JiaLiPassion force-pushed the zone-bazel-ref branch 2 times, most recently from bdb03f8 to 411bf9d Compare October 9, 2019 15:30

JiaLiPassion requested a review from alexeagle October 9, 2019 15:35

alexeagle reviewed Oct 9, 2019

View reviewed changes

JiaLiPassion force-pushed the zone-bazel-ref branch from 411bf9d to 4920e4c Compare October 9, 2019 22:28

alexeagle reviewed Oct 11, 2019

View reviewed changes

JiaLiPassion force-pushed the zone-bazel-ref branch 2 times, most recently from c90feea to a2e9082 Compare October 24, 2019 15:38

alexeagle reviewed Oct 25, 2019

View reviewed changes

JiaLiPassion removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Oct 28, 2019

atscott added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Nov 5, 2019

JiaLiPassion force-pushed the zone-bazel-ref branch 2 times, most recently from 934eb84 to ae92649 Compare November 5, 2019 22:37

JiaLiPassion removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Nov 5, 2019

JiaLiPassion force-pushed the zone-bazel-ref branch 2 times, most recently from 7692c80 to 42dcb7e Compare November 5, 2019 22:51

build: reference zone.js from source directly instead of npm.

203e24d

Close angular#32482

JiaLiPassion force-pushed the zone-bazel-ref branch from 42dcb7e to 203e24d Compare November 5, 2019 23:13

atscott closed this in 8c6fb17 Nov 6, 2019

atscott pushed a commit that referenced this pull request Nov 6, 2019

build: reference zone.js from source directly instead of npm. (#33046)

88eefd8

Close #32482 PR Close #33046

mohaxspb pushed a commit to mohaxspb/angular that referenced this pull request Nov 7, 2019

build: reference zone.js from source directly instead of npm. (angula…

98965b8

…r#33046) Close angular#32482 PR Close angular#33046

mohaxspb pushed a commit to mohaxspb/angular that referenced this pull request Nov 7, 2019

build: reference zone.js from source directly instead of npm. (angula…

9379d6d

…r#33046) Close angular#32482 PR Close angular#33046

angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: reference zone.js directly from source instead of npm #33046

build: reference zone.js directly from source instead of npm #33046

JiaLiPassion commented Oct 9, 2019

alexeagle Oct 9, 2019

JiaLiPassion Oct 9, 2019

alexeagle Oct 11, 2019

JiaLiPassion Oct 26, 2019

alexeagle Oct 11, 2019

JiaLiPassion Oct 12, 2019

alexeagle Oct 18, 2019

JiaLiPassion Oct 24, 2019

alexeagle Oct 25, 2019

JiaLiPassion Oct 26, 2019

alexeagle Oct 25, 2019

JiaLiPassion Oct 26, 2019

mhevery commented Nov 5, 2019

mhevery commented Nov 5, 2019 •

edited

Loading

atscott commented Nov 5, 2019

JiaLiPassion commented Nov 5, 2019

atscott commented Nov 6, 2019

angular-automatic-lock-bot bot commented Dec 7, 2019

build: reference zone.js directly from source instead of npm #33046

build: reference zone.js directly from source instead of npm #33046

Conversation

JiaLiPassion commented Oct 9, 2019

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhevery commented Nov 5, 2019

mhevery commented Nov 5, 2019 • edited Loading

atscott commented Nov 5, 2019

JiaLiPassion commented Nov 5, 2019

atscott commented Nov 6, 2019

angular-automatic-lock-bot bot commented Dec 7, 2019

mhevery commented Nov 5, 2019 •

edited

Loading