Commit Graph

16513 Commits

Author SHA1 Message Date
Filipe Silva 891708cfc9 build: update to Angular CLI 9.0.0-rc.3 (#33955)
Followup to https://github.com/angular/angular/pull/33337

PR Close #33955
2019-11-21 09:17:18 -08:00
Andrew Kushnir fc2f6b8456 fix(ivy): wrap functions from "providers" in parentheses in Closure mode (#33609)
Due to the fact that Tsickle runs between analyze and transform phases in Angular, Tsickle may transform nodes (add comments with type annotations for Closure) that we captured during the analyze phase. As a result, some patterns where a function is returned from another function may trigger automatic semicolon insertion, which breaks the code (makes functions return `undefined` instead of a function). In order to avoid the problem, this commit updates the code to wrap all functions in some expression ("privders" and "viewProviders") in parentheses. More info can be found in Tsickle source code here: d797426257/src/jsdoc_transformer.ts (L1021)

PR Close #33609
2019-11-20 14:58:35 -08:00
Dyma 11a35e5d14 docs: add learning resource (#33730)
PR Close #33730
2019-11-20 14:56:56 -08:00
Alyssa Nicoll 4ae754a222 docs: Update Alyssa on GDE list (#33538)
PR Close #33538
2019-11-20 14:52:00 -08:00
JoostK b07b6f1d40 fix(ivy): avoid infinite recursion when evaluation source files (#33772)
When ngtsc comes across a source file during partial evaluation, it
would determine all exported symbols from that module and evaluate their
values greedily. This greedy evaluation strategy introduces unnecessary
work and can fall into infinite recursion when the evaluation result of
an exported expression would circularly depend on the source file. This
would primarily occur in CommonJS code, where the `exports` variable can
be used to refer to an exported variable. This variable would be
resolved to the source file itself, thereby greedily evaluating all
exported symbols and thus ending up evaluating the `exports` variable
again. This variable would be resolved to the source file itself,
thereby greedily evaluating all exported symbols and thus ending u
evaluating the `exports` variable again. This variable would be
resolved to the source file itself, thereby greedily evaluating all
exported symbols and thus ending up evaluating the `exports` variable
again. This variable would be resolved to the source file itself,
thereby greedily evaluating all exported symbols and thus ending up
evaluating the `exports` variable again. This went on for some time
until all stack frames were exhausted.

This commit introduces a `ResolvedModule` that delays the evaluation of
its exports until they are actually requested. This avoids the circular
dependency when evaluating `exports`, thereby fixing the issue.

Fix #33734

PR Close #33772
2019-11-20 14:51:37 -08:00
JoostK 70311ebca1 fix(ivy): handle non-standard input/output names in template type checking (#33741)
The template type checker generates code to check directive inputs and
outputs, whose name may contain characters that can not be used as
identifier in TypeScript. Prior to this change, such names would be
emitted into the generated code as is, resulting in invalid code and
unexpected template type check errors.

This commit fixes the bug by representing the potentially invalid names
as string literal instead of raw identifier.

Fixes #33590

PR Close #33741
2019-11-20 14:51:12 -08:00
Andrew Kushnir fd83d9479a fix(ivy): avoid using stale cache in TestBed if module overrides are defined (#33787)
NgModule compilation in JIT mode (that is also used in TestBed) caches module scopes on NgModule defs (using `transitiveCompileScopes` field). Module overrides (defined via TestBed.overrideModule) may invalidate this data by adding/removing items in `declarations` list. This commit forces TestBed to recalculate transitive scopes in case module overrides are present, so TestBed always gets the most up-to-date information.

PR Close #33787
2019-11-20 14:50:27 -08:00
Judy Bogart 87994d2c03 docs: add api doc to sub-packages (#33801)
PR Close #33801
2019-11-20 14:48:50 -08:00
Igor Minar 55748dbc55 fix(core): allow css custom variables/properties in the style sanitizer (#33841)
This change enables "var(--my-var)" to pass through the style sanitizer.

After consulation with our security team, allowing these doesn't create
new attack vectors, so the sanitizer doesn't need to strip them.

Fixes parts of #23485 related to the sanitizer, other use cases discussed
there related to binding have been addressed via other changes to the
class and style handling in the runtime.

Closes #23485

PR Close #33841
2019-11-20 14:47:59 -08:00
Igor Minar 6a5475f65b style(core): improve readability of style_sanizer(_spec).ts (#33841)
PR Close #33841
2019-11-20 14:47:59 -08:00
Andrew Scott 5af3bd4728 perf(ivy): R3TestBed - Do not process NgModuleDefs that have already been processed (#33863)
PR Close #33863
2019-11-20 14:47:42 -08:00
Judy Bogart e13aa65f49 docs: add doc reference to npm package readme (#33911)
PR Close #33911
2019-11-20 14:46:23 -08:00
horn 7b4853bae4 fix(ivy): reset style property using ngStyle fix (#33920)
PR Close #33920
2019-11-20 14:46:00 -08:00
Pawel Kozlowski 51cee50ee3 test(ivy): non-regression test for ViewContainerRef queried on ng-container (#33939)
Closes #31971

PR Close #33939
2019-11-20 14:45:43 -08:00
George Kalpakas 27562e92db fix(docs-infra): fix StackBlitz and zipped `http` examples (#33941)
Previously, the generated StackBlitz examples as well as the
corresponding downloadable zips for the `http` guide examples were not
correct and thus trying to run the app and/or tests would fail.

This commit fixes the examples:

- Replace `TestBed.inject()` (which was [introduced in v9][1]) with
  `TestBed.get()` (which is available in v8 used in the examples).
  (NOTE: The examples will soon be updated to v9 (as part of
  [FW-1609][2] and switched back to `TestBed.inject()` then.)

- Include `src/app/heroes/hero.ts` in the zip, because it is referenced
  by some of the other files and the compilation fails without it.

- Ensure `src/main-specs.ts` is not included in the zip that does not
  include the tests. Including the file broke the app, because there is
  logic in our zip-builder that renamed `main-*.ts` files to `main.ts`
  and thus `main-specs.ts` ended up overwriting the actual `main.ts`.

[1]: https://next.angular.io/guide/deprecations#angularcoretesting
[2]: https://angular-team.atlassian.net/browse/FW-1609

Fixes #33874
Fixes #33945

PR Close #33941
2019-11-20 14:44:31 -08:00
Alex Rickabaugh d78d29ff8c docs: release notes for the v9.0.0-rc.3 release 2019-11-20 14:28:48 -08:00
Alex Rickabaugh 08a4f10ee7 fix(ivy): move setClassMetadata calls into a pure iife (#33337)
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
https://github.com/angular/angular-cli/pull/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
2019-11-20 12:55:58 -08:00
Alex Rickabaugh b54ed980ed fix(ivy): retain JIT metadata unless JIT mode is explicitly disabled (#33671)
NgModules in Ivy have a definition which contains various different bits
of metadata about the module. In particular, this metadata falls into two
categories:

* metadata required to use the module at runtime (for bootstrapping, etc)
in AOT-only applications.
* metadata required to depend on the module from a JIT-compiled app.

The latter metadata consists of the module's declarations, imports, and
exports. To support JIT usage, this metadata must be included in the
generated code, especially if that code is shipped to NPM. However, because
this metadata preserves the entire NgModule graph (references to all
directives and components in the app), it needs to be removed during
optimization for AOT-only builds.

Previously, this was done with a clever design:

1. The extra metadata was added by a function called `setNgModuleScope`.
A call to this function was generated after each NgModule.
2. This function call was marked as "pure" with a comment and used
`noSideEffects` internally, which causes optimizers to remove it.

The effect was that in dev mode or test mode (which use JIT), no optimizer
runs and the full NgModule metadata was available at runtime. But in
production (presumably AOT) builds, the optimizer runs and removes the JIT-
specific metadata.

However, there are cases where apps that want to use JIT in production, and
still make an optimized build. In this case, the JIT-specific metadata would
be erroneously removed. This commit solves that problem by adding an
`ngJitMode` global variable which guards all `setNgModuleScope` calls. An
optimizer can be configured to statically define this global to be `false`
for AOT-only builds, causing the extra metadata to be stripped.

A configuration for Terser used by the CLI is provided in `tooling.ts` which
sets `ngJitMode` to `false` when building AOT apps.

PR Close #33671
2019-11-20 12:55:43 -08:00
Alex Rickabaugh eb6975acaf fix(ivy): don't infer template context types when in full mode (#33537)
The Ivy template type-checker is capable of inferring the type of a
structural directive (such as NgForOf<T>). Previously, this was done with
fullTemplateTypeCheck: true, even if strictTemplates was false. View Engine
previously did not do this inference, and so this causes breakages if the
type of the template context is not what the user expected.

In particular, consider the template:

```html
<div *ngFor="let user of users as all">
  {{user.index}} out of {{all.length}}
</div>
```

As long as `users` is an array, this seems reasonable, because it appears
that `all` is an alias for the `users` array. However, this is misleading.

In reality, `NgForOf` is rendered with a template context that contains
both a `$implicit` value (for the loop variable `user`) as well as a
`ngForOf` value, which is the actual value assigned to `all`. The type of
`NgForOf`'s template context is `NgForContext<T>`, which declares `ngForOf`'s
type to be `NgIterable<T>`, which does not have a `length` property (due to
its incorporation of the `Iterable` type).

This commit stops the template type-checker from inferring template context
types unless strictTemplates is set (and strictInputTypes is not disabled).

Fixes #33527.

PR Close #33537
2019-11-20 11:47:42 -08:00
Alex Rickabaugh 97fbdab3b8 fix(ivy): report watch mode diagnostics correctly (#33862)
This commit changes the reporting of watch mode diagnostics for ngtsc to use
the same formatting as non-watch mode diagnostics. This prints rich and
contextual errors even in watch mode, which previously was not the case.

Fixes #32213

PR Close #33862
2019-11-20 11:46:02 -08:00
Alex Rickabaugh 4be8929844 fix(ivy): always re-analyze the program during incremental rebuilds (#33862)
Previously, the ngtsc compiler attempted to reuse analysis work from the
previous program during an incremental build. To do this, it had to prove
that the work was safe to reuse - that no changes made to the new program
would invalidate the previous analysis.

The implementation of this had a significant design flaw: if the previous
program had errors, the previous analysis would be missing significant
information, and the dependency graph extracted from it would not be
sufficient to determine which files should be re-analyzed to fill in the
gaps. This often meant that the build output after an error was resolved
would be wholly incorrect.

This commit switches ngtsc to take a simpler approach to incremental
rebuilds. Instead of attempting to reuse prior analysis work, the entire
program is re-analyzed with each compilation. This is actually not as
expensive as one might imagine - analysis is a fairly small part of overall
compilation time.

Based on the dependency graph extracted during this analysis, the compiler
then can make accurate decisions on whether to emit specific files. A new
suite of tests is added to validate behavior in the presence of source code
level errors.

This new approach is dramatically simpler than the previous algorithm, and
should always produce correct results for a semantically correct program.s

Fixes #32388
Fixes #32214

PR Close #33862
2019-11-20 11:46:02 -08:00
Alex Rickabaugh cf9aa4fd14 test(ivy): driveDiagnostics() works incrementally (#33862)
PR Close #33862
2019-11-20 11:46:02 -08:00
George Kalpakas 9e45203679 test(docs-infra): more thoroughly clean up after `ScrollService` tests (#33937)
By clearing `sessionStorage` and unsubscribing from `Location` events,
after each test, we reduce the possibility of potential
[spooky action at a distance][1]-type of failures in the future.

This does not have an impact on the actual app, since `ScrollService` is
currently expected to live throughout the lifetime of the app. Still,
unsubscribing from `Location` events keeps the code consistent with how
other `ScrollService` listeners are handled (e.g. for `window` events).

[1]: https://en.wikipedia.org/wiki/Action_at_a_distance_(computer_programming)

PR Close #33937
2019-11-20 11:04:12 -08:00
George Kalpakas b7fd86ec50 test(docs-infra): destroy all `ScrollService` instances after each test (#33937)
`ScrollService` subscribes to global `window` events and mutates global
state in the listener (e.g. read/write values from/to `sessionStorage`).
Therefore, we need to always call its `ngOnDestroy()` method to
unsubscribe from these events after each test.

In f69c6e204, a new testcase was introduced that was not destroyed. As a
result, random failures started to randomly happen in other, unrelated
tests ([example CI failure][1]).

This commit fixes this by ensuring all `ScrollService` instances are
destroyed after each tests (provided that they are created with the
`createScrollService()` helper).

[1]: https://circleci.com/gh/angular/angular/533298

PR Close #33937
2019-11-20 11:04:12 -08:00
Greg Magolan a959fae66e build: add ngJitMode to ng_rollup_bundle terser config (#33865)
This adds a `tools/ng_rollup_bundle/terser_config.json` file to override the default terser_minified config provided by the rule. After this change, the layer violation in rules_nodejs can be fixed by removing `"global_defs": {"ngDevMode": false, "ngI18nClosureMode": false},` from `terser_config.default.json` in rules_nodejs.

Change requested by Alex Rickabaugh in https://github.com/bazelbuild/rules_nodejs/pull/1338.

PR Close #33865
2019-11-20 10:29:46 -08:00
Alex Rickabaugh bb290cefae fix(core): make QueryList implement Iterable in the type system (#33536)
Originally, QueryList implemented Iterable and provided a Symbol.iterator
on its prototype. This caused issues with tree-shaking, so QueryList was
refactored and the Symbol.iterator added in its constructor instead. As
part of this change, QueryList no longer implemented Iterable directly.

Unfortunately, this meant that QueryList was no longer assignable to
Iterable or, consequently, NgIterable. NgIterable is used for NgFor's input,
so this meant that QueryList was not usable (in a type sense) for NgFor
iteration. View Engine's template type checking would not catch this, but
Ivy's did.

As a fix, this commit adds the declaration (but not the implementation) of
the Symbol.iterator function back to QueryList. This has no runtime effect,
so it doesn't affect tree-shaking of QueryList, but it ensures that
QueryList is assignable to NgIterable and thus usable with NgFor.

Fixes #29842

PR Close #33536
2019-11-19 13:43:53 -08:00
Alex Rickabaugh 850aee2448 fix(ivy): emit fs-relative paths when rootDir(s) aren't in effect (#33828)
Previously, the compiler assumed that all TS files logically within a
project existed under one or more "root directories". If the TS compiler
option `rootDir` or `rootDirs` was set, they would dictate the root
directories in use, otherwise the current directory was used.

Unfortunately this assumption was unfounded - it's common for projects
without explicit `rootDirs` to import from files outside the current
working directory. In such cases the `LogicalProjectStrategy` would attempt
to generate imports into those files, and fail. This would lead to no
`ReferenceEmitStrategy` being able to generate an import, and end in a
compiler assertion failure.

This commit introduces a new strategy to use when there are no `rootDirs`
explicitly present, the `RelativePathStrategy`. It uses simpler, filesystem-
relative paths to generate imports, even to files above the current working
directory.

Fixes #33659
Fixes #33562

PR Close #33828
2019-11-19 12:41:24 -08:00
Alex Rickabaugh 51720745dd test(ivy): support chdir() on the compiler's filesystem abstraction (#33828)
This commit adds the ability to change directories using the compiler's
internal filesystem abstraction. This is a prerequisite for writing tests
which are sensitive to the current working directory.

In addition to supporting the `chdir()` operation, this commit also fixes
`getDefaultLibLocation()` for mock filesystems to not assume `node_modules`
is in the current directory, but to resolve it similarly to how Node does
by progressively looking higher in the directory tree.

PR Close #33828
2019-11-19 12:41:24 -08:00
Pawel Kozlowski 6bf2531b19 fix(ivy): properly insert views before ng-container with injected ViewContainerRef (#33853)
PR Close #33853
2019-11-19 11:56:43 -08:00
Pawel Kozlowski e698d355c1 refactor(ivy): stricter TNode.inputs typing (#33798)
TNode.inputs are initialised during directives resolution now so we know early
if a node has directives with inputs or no. We don't need to use undefined value
as an indicator that inputs were not resolved yet.

PR Close #33798
2019-11-19 11:56:00 -08:00
Pawel Kozlowski da0c372fdf perf(ivy): don't store public input names in two places (#33798)
Before this change a public name of a directive's input
was stored in 2 places:
- as a key of an object on TNode.index;
- as a value of PropertyAliasValue at the index 1

This PR changes the data structure so the public name is stored
only once as a key on TNode.index. This saves one array entry
for each and every directive input.

PR Close #33798
2019-11-19 11:55:59 -08:00
Pawel Kozlowski 5aec1798eb perf(ivy): add micro-benchmark focused on directive input update (#33798)
PR Close #33798
2019-11-19 11:55:59 -08:00
Kristiyan Kostadinov 8a052dc858 perf(ivy): chain styling instructions (#33837)
Adds support for chaining of `styleProp`, `classProp` and `stylePropInterpolateX` instructions whenever possible which should help generate less code. Note that one complication here is for `stylePropInterpolateX` instructions where we have to break into multiple chains if there are other styling instructions inbetween the interpolations which helps maintain the execution order.

PR Close #33837
2019-11-19 11:44:29 -08:00
ajitsinghkaler f69c6e204a fix(docs-infra): do not break when cookies are disabled in the browser (#33829)
Whenever cookies are disabled in the browser, `window.sessionStorage` is not avaialable to the app (i.e. even trying to access `window.sessionStorage` throws an error).

To avoid breaking the app, we use a no-op `Storage` implementation if `window.sessionStorage` is not available.

Fixes #33795

PR Close #33829
2019-11-19 11:42:11 -08:00
Pawel Kozlowski 8555d51bc7 perf(ivy): extract template's instruction first create pass processing (#33856)
This refactorings clearly separates the first and subsequent creation execution
of the `template` instruction. This approach has the following benefits:
- it is clear what happens during the first vs. subsequent executions;
- we can avoid several memory reads and checks after the first creation pass
(there is measurable performance improvement on various benchmarks);
- the template instructions becomes smaller and should become a candidate
for optimisations / inlining faster;

PR Close #33856
2019-11-19 11:41:49 -08:00
Pawel Kozlowski ad4300f52b refactor(ivy): limit reads from TView.consts (#33856)
PR Close #33856
2019-11-19 11:41:49 -08:00
Lars Gyrup Brink Nielsen 3557849371 docs: correct lifecycle hooks feature example (#33886)
PR Close #33886
2019-11-19 11:40:59 -08:00
Pete Bacon Darwin be58159550 build: ensure package-builder finds bazel (#33904)
Since `scripts/package-builder.js` gets run from the
`aio` folder sometimes, it is important to set the cwd
for the command that finds the bazel bin directory.

PR Close #33904
2019-11-19 11:40:19 -08:00
Judy Bogart 5c23ea71c4 docs: add version requirement for routing option (#33909)
PR Close #33909
2019-11-19 11:40:03 -08:00
Greg Magolan 288009d6fb build: remove rules_nodejs legacy rollup_bundle patch (#33915)
We no longer depend on the legacy rollup_bundle rule so we don't require this patch

PR Close #33915
2019-11-19 11:39:49 -08:00
Alex Rickabaugh 29b8666b10 fix(ngcc): properly detect origin of constructor param types (#33901)
The ReflectionHost supports enumeration of constructor parameters, and one
piece of information it returns describes the origin of the parameter's
type. Parameter types come in two flavors: local (the type is not imported
from anywhere) or non-local (the type comes via an import).

ngcc incorrectly classified all type parameters as 'local', because in the
source files that ngcc processes the type parameter is a real ts.Identifer.
However, that identifier may still have come from an import and thus might
be non-local.

This commit changes ngcc's ReflectionHost(s) to properly recognize and
report these non-local type references.

Fixes #33677

PR Close #33901
2019-11-19 11:38:33 -08:00
Joey Perrott c68200cf42 build: remove rollup-rule temp settings (#33908)
PR Close #33908
2019-11-18 16:02:25 -08:00
Joey Perrott 90c3d98dd9 build: force RBE runs to use linux remote executors (#33902)
PR Close #33902
2019-11-18 16:02:10 -08:00
Pawel Kozlowski c44415494a refactor(ivy): separate first creation pass in the elementContainerStart instruction (#33894)
PR Close #33894
2019-11-18 16:01:54 -08:00
Pawel Kozlowski 96499c898f refactor(ivy): separate first creation pass in the text instruction (#33894)
PR Close #33894
2019-11-18 16:01:54 -08:00
Alan Agius ffa3936b4c fix(bazel): add terser as an optional peer dependency (#33891)
`ng_package` rule has an implicitly optional depedency on terser a48573efe8/packages/bazel/src/ng_package/ng_package.bzl (L36)

When using this rule without terser being available we get the below error;
```
ERROR: /home/circleci/ng/modules/express-engine/BUILD.bazel:22:1: every rule of type ng_package implicitly depends upon the target '@npm//terser/bin:terser', but this target could not be found because of: no such package '@npm//terser/bin': BUILD file not found in directory 'terser/bin' of external repository @npm. Add a BUILD file to a directory to mark it as a package.
ERROR: Analysis of target '//modules/express-engine:npm_package' failed; build aborted: no such package '@npm//terser/bin': BUILD file not found in directory 'terser/bin' of external repository @npm. Add a BUILD file to a directory to mark it as a package.
```

PR Close #33891
2019-11-18 16:01:33 -08:00
Pete Bacon Darwin a6247aafa1 fix(ivy): i18n - support "\", "`" and "${" sequences in i18n messages (#33820)
Since i18n messages are mapped to `$localize` tagged template strings,
the "raw" version must be properly escaped. Otherwise TS will throw an
error such as:

```
Error: Debug Failure. False expression: Expected argument 'text' to be the normalized (i.e. 'cooked') version of argument 'rawText'.
```

This commit ensures that we properly escape these raw strings before creating
TS AST nodes from them.

PR Close #33820
2019-11-18 16:00:22 -08:00
Pete Bacon Darwin 62f7d0fe5c fix(ivy): i18n - ensure that colons in i18n metadata are not rendered (#33820)
The `:` char is used as a metadata marker in `$localize` messages.
If this char appears in the metadata it must be escaped, as `\:`.
Previously, although the `:` char was being escaped, the TS AST
being generated was not correct and so it was being output double
escaped, which meant that it appeared in the rendered message.

As of TS 3.6.2 the "raw" string can be specified when creating tagged
template AST nodes, so it is possible to correct this.

PR Close #33820
2019-11-18 16:00:22 -08:00
Pete Bacon Darwin 74e6d379d8 build: make ivy-i18n/debug-test.sh executable (#33820)
PR Close #33820
2019-11-18 16:00:22 -08:00
Pawel Kozlowski 49c9f782ab fix(ivy): properly insert views into ViewContainerRef injected by querying <ng-container> (#33816)
When asking for a ViewContainerRef on <ng-container> we do reuse <ng-container> comment
node as a LContainer's anachor. Before this fix the act of re-using a <ng-container>'s
comment node would result in this comment node being re-appended to the DOM in the wrong
place. With the fix in this PR we make sure that re-using <ng-container>'s comment node
doesn't result in unwanted DOM manipulation (ng-gontainer's comment node is already part
of the DOM and doesn't have to be re-created / re-appended).

PR Close #33816
2019-11-18 16:00:00 -08:00