test(ivy): add render3 integration tests #21557

marclaval · 2018-01-16T16:34:54Z

This PR adds two Hello World integration tests with the Render3 engine. They don't use the compiler yet, but they will be useful to track the size of a small application.
They implement the same Hello World application as the other integration tests.

The first one is using Rollup to create the bundle and targets the standard packages of @angular. Since they are not correctly tree-shakable, the bundle ends up at 56.6kB of minified code (15.3kB after gzip).

The second is a bit more hacky. It uses Closure Compiler and the Angular source code compiled to ES2015, in order to remove as much unused code as possible. With this setup, the bundle is only 6.6kB minified (3.1kB after gzip).

IgorMinar

overall lgtm, just needs a bit of cleanup.

IgorMinar · 2018-01-18T19:37:07Z

integration/_payload-limits.json

+  "hello_world__render3__closure": {
+    "master": {
+      "gzip7": {
+        "bundle": 3138


please remove limits for gzip7 and gzip9. going forward we want to break builds only if uncompressed size changes by more than 1%

IgorMinar · 2018-01-18T19:38:35Z

integration/hello_world__render3__closure/clean_ngDevMode.sh

@@ -0,0 +1,2 @@
+find built/packages/core/src/render3/* -name '*.js' -exec sed  -i '' -e "s/import '.\/ng_dev_mode';//g" {} \;


can you document what this does and why do we do it this way?

I thought closure had a way to set the compilation constant via config so I'm not sure why we do this kind of search&replace. if there is a good reason for it, please document it in this file.

This is workaround for google/closure-compiler#1601 Can you include the link in a comment?

Could you see if you can get it work with

-D "ngDevMode$$module$built$packages$core$src$render3$ng_dev_mode=false" -D "ngDevMode=false"

This should set the closure compiler vars and tree shake them, but I don't seem to be able to get it to work.

Comment added. Due to this bug, I couldn't make it work either.

IgorMinar · 2018-01-18T19:39:16Z

integration/hello_world__render3__closure/package.json

+{
+  "name": "angular-integration",
+  "version": "0.0.0",
+  "license": "MIT",


add "private: true" plz

IgorMinar · 2018-01-18T19:41:09Z

integration/hello_world__render3__closure/tsconfig.json

+  "compilerOptions": {
+    "module": "es2015",
+    "moduleResolution": "node",
+    "strictNullChecks": true,


replace with "strict": true

IgorMinar · 2018-01-18T19:41:24Z

integration/hello_world__render3__rollup/e2e/tsconfig.json

+    "outDir": "../built/e2e",
+    "types": ["jasmine"],
+    // TODO(alexeagle): was required for Protractor 4.0.11
+    "skipLibCheck": true


add "strict": true ?

IgorMinar · 2018-01-18T19:41:35Z

integration/hello_world__render3__rollup/package.json

+{
+  "name": "angular-integration",
+  "version": "0.0.0",
+  "license": "MIT",


private true

IgorMinar · 2018-01-18T19:41:52Z

integration/hello_world__render3__rollup/tsconfig.json

+  "compilerOptions": {
+    "module": "es2015",
+    "moduleResolution": "node",
+    "strictNullChecks": true,


replace with "strict": true

mhevery · 2018-01-18T21:24:52Z

integration/hello_world__render3__closure/clean_ngDevMode.sh

@@ -0,0 +1,2 @@
+find built/packages/core/src/render3/* -name '*.js' -exec sed  -i '' -e "s/import '.\/ng_dev_mode';//g" {} \;


This is workaround for google/closure-compiler#1601 Can you include the link in a comment?

mhevery · 2018-01-18T21:40:30Z

integration/hello_world__render3__closure/closure.conf

+--rewrite_polyfills=false
+--jscomp_off=checkVars
+
+node_modules/zone.js/dist/zone_externs.js


I am confused why do we need zone here. Render3 should have no dependency on it.

Removed the leftover

mhevery · 2018-01-18T21:41:23Z

integration/hello_world__render3__closure/package.json

+  "dependencies": {
+    "rxjs": "file:../../node_modules/rxjs",
+    "typescript": "file:../../node_modules/typescript",
+    "zone.js": "file:../../node_modules/zone.js"


Why is there dep on rxjs if we don't need it? (Also zone.js)?

Why is there NO dep on core which is needed?

Removed rxjs

For the Angular source code, this scenario doesn't use distributed packages but just the source transpiled to ES2015. It is copied into built folder before the build.

marclaval · 2018-01-19T10:42:04Z

@IgorMinar @mhevery thanks for the reviews, I've updated the PR with your comments, please take another look

IgorMinar

lgtm

mhevery · 2018-01-19T20:06:55Z

http://test/OCL:182569009:BASE:182569051:1516392364907:e8699091

PR Close angular#21557

angular-automatic-lock-bot · 2019-09-13T05:21:28Z

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 Jan 16, 2018

marclaval requested a review from mhevery January 16, 2018 17:05

marclaval added action: review The PR is still awaiting reviews from at least one requested reviewer comp: ivy labels Jan 16, 2018

marclaval requested a review from IgorMinar January 16, 2018 17:05

marclaval force-pushed the hwRender3 branch from 1b5dbaf to 953e384 Compare January 18, 2018 11:07

IgorMinar approved these changes Jan 18, 2018

View reviewed changes

IgorMinar added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 18, 2018

mhevery reviewed Jan 18, 2018

View reviewed changes

marclaval force-pushed the hwRender3 branch from 953e384 to 2bd9cb1 Compare January 19, 2018 10:37

marclaval added action: review The PR is still awaiting reviews from at least one requested reviewer and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Jan 19, 2018

marclaval force-pushed the hwRender3 branch from 2bd9cb1 to 2eb1744 Compare January 19, 2018 10:44

test(ivy): add render3 integration tests

2eb1744

IgorMinar approved these changes Jan 19, 2018

View reviewed changes

IgorMinar added target: patch This PR is targeted for the next patch release 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 labels Jan 19, 2018

mhevery added the g3_presubmit label Jan 19, 2018

mhevery closed this in a0dc0a2 Jan 19, 2018

jbogarthyde pushed a commit to jbogarthyde/angular that referenced this pull request Feb 23, 2018

test(ivy): add render3 integration tests (angular#21557)

a8c1b81

PR Close angular#21557

leo6104 pushed a commit to leo6104/angular that referenced this pull request Mar 25, 2018

test(ivy): add render3 integration tests (angular#21557)

c66778f

PR Close angular#21557

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(ivy): add render3 integration tests #21557

test(ivy): add render3 integration tests #21557

marclaval commented Jan 16, 2018

IgorMinar left a comment

IgorMinar Jan 18, 2018

marclaval Jan 19, 2018

IgorMinar Jan 18, 2018

mhevery Jan 18, 2018

mhevery Jan 18, 2018

marclaval Jan 19, 2018

IgorMinar Jan 18, 2018

marclaval Jan 19, 2018

IgorMinar Jan 18, 2018

marclaval Jan 19, 2018

IgorMinar Jan 18, 2018

marclaval Jan 19, 2018

IgorMinar Jan 18, 2018

marclaval Jan 19, 2018

IgorMinar Jan 18, 2018

marclaval Jan 19, 2018

mhevery Jan 18, 2018

mhevery Jan 18, 2018

marclaval Jan 19, 2018

mhevery Jan 18, 2018

marclaval Jan 19, 2018

marclaval commented Jan 19, 2018

IgorMinar left a comment

mhevery commented Jan 19, 2018

angular-automatic-lock-bot bot commented Sep 13, 2019

		@@ -0,0 +1,2 @@
		find built/packages/core/src/render3/* -name '*.js' -exec sed -i '' -e "s/import '.\/ng_dev_mode';//g" {} \;

test(ivy): add render3 integration tests #21557

test(ivy): add render3 integration tests #21557

Conversation

marclaval commented Jan 16, 2018

IgorMinar left a 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

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

marclaval commented Jan 19, 2018

IgorMinar left a comment

Choose a reason for hiding this comment

mhevery commented Jan 19, 2018

angular-automatic-lock-bot bot commented Sep 13, 2019