Skip to content

Update ivy set calls#16228

Merged
vikerman merged 3 commits into
angular:masterfrom
filipesilva:update-ivy-set-calls
Nov 21, 2019
Merged

Update ivy set calls#16228
vikerman merged 3 commits into
angular:masterfrom
filipesilva:update-ivy-set-calls

Conversation

@filipesilva
Copy link
Copy Markdown
Contributor

No description provided.

@filipesilva
Copy link
Copy Markdown
Contributor Author

This PR is blocked by angular/angular#33671 and angular/angular#33337 being merged and released. Afterwards, this PR should be updated to use the new FW version.

Copy link
Copy Markdown
Contributor

@IgorMinar IgorMinar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per our discussion on angular/angular#33337 this change LGTM

@@ -304,7 +303,35 @@ function isAssignmentExpressionTo(exprStmt: ts.ExpressionStatement, name: string
}

function isIvyPrivateCallExpression(exprStmt: ts.ExpressionStatement) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: The IIFE structure could technically be different depending on any intermediate processing steps. The CLI's fixed configuration will prevent this but other usage may cause changes. An unbounded number of parenthesized expressions are also technically possible.

Examples: !function(){}() or (function(){}()) or ((function(){})())

alxhub added a commit to IgorMinar/angular that referenced this pull request Nov 20, 2019
This commit transforms the setClassMetadata calls generated by ngtsc from:

```typescript
/*@__PURE__*/ setClassMetadata(...);
```

to:

```typescript
/*@__PURE__*/ (function() {
  setClassMetadata(...);
})();
```

Without the IIFE, terser won't remove these function calls because the
function calls have arguments that themselves are function calls or other
impure expressions. In order to make the whole block be DCE-ed by terser,
we wrap it into IIFE and mark the IIFE as pure.

It should be noted that this change doesn't have any impact on CLI* with
build-optimizer, which removes the whole setClassMetadata block within
the webpack loader, so terser or webpack itself don't get to see it at
all. This is done to prevent cross-chunk retention issues caused by
webpack's internal module registry.

* actually we do expect a short-term size regression while
angular/angular-cli#16228
is merged and released in the next rc of the CLI. But long term this
change does nothing to CLI + build-optimizer configuration and is done
primarly to correct the seemingly correct but non-function PURE annotation
that builds not using build-optimizer could rely on.
alxhub added a commit to angular/angular that referenced this pull request Nov 20, 2019
This commit transforms the setClassMetadata calls generated by ngtsc from:

```typescript
/*@__PURE__*/ setClassMetadata(...);
```

to:

```typescript
/*@__PURE__*/ (function() {
  setClassMetadata(...);
})();
```

Without the IIFE, terser won't remove these function calls because the
function calls have arguments that themselves are function calls or other
impure expressions. In order to make the whole block be DCE-ed by terser,
we wrap it into IIFE and mark the IIFE as pure.

It should be noted that this change doesn't have any impact on CLI* with
build-optimizer, which removes the whole setClassMetadata block within
the webpack loader, so terser or webpack itself don't get to see it at
all. This is done to prevent cross-chunk retention issues caused by
webpack's internal module registry.

* actually we do expect a short-term size regression while
angular/angular-cli#16228
is merged and released in the next rc of the CLI. But long term this
change does nothing to CLI + build-optimizer configuration and is done
primarly to correct the seemingly correct but non-function PURE annotation
that builds not using build-optimizer could rely on.

PR Close #33337
alxhub added a commit to angular/angular that referenced this pull request Nov 20, 2019
This commit transforms the setClassMetadata calls generated by ngtsc from:

```typescript
/*@__PURE__*/ setClassMetadata(...);
```

to:

```typescript
/*@__PURE__*/ (function() {
  setClassMetadata(...);
})();
```

Without the IIFE, terser won't remove these function calls because the
function calls have arguments that themselves are function calls or other
impure expressions. In order to make the whole block be DCE-ed by terser,
we wrap it into IIFE and mark the IIFE as pure.

It should be noted that this change doesn't have any impact on CLI* with
build-optimizer, which removes the whole setClassMetadata block within
the webpack loader, so terser or webpack itself don't get to see it at
all. This is done to prevent cross-chunk retention issues caused by
webpack's internal module registry.

* actually we do expect a short-term size regression while
angular/angular-cli#16228
is merged and released in the next rc of the CLI. But long term this
change does nothing to CLI + build-optimizer configuration and is done
primarly to correct the seemingly correct but non-function PURE annotation
that builds not using build-optimizer could rely on.

PR Close #33337
@angular-automatic-lock-bot
Copy link
Copy Markdown

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.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Dec 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants