When analyzing components, directives, etc we capture its base class.
Previously this assumed that the code is in TS format, which is not
always the case (e.g. ngcc).
Now this code is replaced with a call to
`ReflectionHost.getBaseClassExpression()`, which abstracts the work
of finding the base class.
PR Close#31544
Previously the last file-system being tested was left as the current
file-system. Now it is reset to an `InvalidFileSystem` to ensure future
tests are not affected.
PR Close#31544
Moves all google3 migration tslint rules into a single directory.
This makes it easier to wire up multiple migration rules in
google3 without having to update the rule directories each time
a new migration is available.
PR Close#30956
Introduces a new migration schematic for adding the "@Injectable()"
decorator to provider classes which are currently not migrated. Previously
in ViewEngine, classes which are declared as providers sometimes don't
require the "@Injectable()" decorator
(e.g. https://stackblitz.com/edit/angular-hpo7gw)
With Ivy, provider classes need to be explicitly decorated with
the "@Injectable()" decorator if they are declared as providers
of a given module. This commit introduces a migration schematic
which automatically adds the explicit decorator to places where
the decorator is currently missing.
The migration logic is designed in a CLI devkit and TSlint agnostic
way so that we can also have this migration run as part of a public
CLI migration w/ `ng update`. This will be handled as part of a follow-up to reiterate on console output etc.
Resolves FW-1371
PR Close#30956
The way the `ComponentFactory.create` is set up at the moment is that if a `rootSelectorOrNode` is passed in, the root context will be injected instead of creating dedicated one for the component. As far as I can tell, there doesn't seem to be a reason to do this and nothing seems to break because of it. These changes switch to always create the root context.
PR Close#31661
In ViewEngine nodes that were inserted through `Renderer2` would also be picked up by `DebugNode.query` and `DebugNode.queryAll`. This worked because everything in ViewEngine went through `Renderer2` and `DebugRenderer2` in dev mode which was able to keep track of the child nodes as they're being inserted. This no longer works in Ivy, because we don't use `DebugRenderer2` and debug nodes work a little differently. These changes work around the issue by walking the DOM as the logical tree is being walked and looking for matches. Note that this is __not__ optimal, because we're walking similar trees multiple times. ViewEngine could do it more efficiently, because all the insertions go through Renderer2, however that's not the case in Ivy. This approach is being used because:
1. Matching the ViewEngine behavior would mean potentially introducing a depedency from `Renderer2` to Ivy which could bring Ivy code into ViewEngine.
2. We would have to make `Renderer3` "know" about debug nodes.
3. It allows us to capture nodes that were inserted directly via the DOM.
PR Close#31716
This commit updates the `_clone` function of the `_ApplySourceSpanTransformer` class, where the for-in loop was used, resulting in copying from prototype to own properties, thus consuming more memory. Prior to NodeJS 12 (V8 versions before 7.4) there was an optimization that was improving the situation and since that logic was removed in favor of other optimizations, the situation with memory consumption caused by the for-in loop got worse. This commit adds a check to make sure we copy only own properties over to cloned object.
Closes#31627.
PR Close#31638
When injecting a `ChangeDetectorRef` into a pipe, the expected result is that the ref will be tied to the component in which the pipe is being used. This works for most cases, however when a pipe is used inside a property binding of a component (see test case as an example), the current `TNode` is pointing to component's host so we end up injecting the inner component's view. These changes fix the issue by only looking up the component view of the `TNode` if the `TNode` is a parent.
This PR resolves FW-1419.
PR Close#31438
In the previous patch () all the existing styling code was turned
off in favor of using the new refactored ivy styling code. This
patch is a follow up patch to that and removes all old, unused
styling code from the render3 directory.
PR Close#31193
This commit fixes a bug where ICU expressions inserted into ngTemplateOutlets
that are inside ngFor blocks would throw an error. We were assuming in view
insertion code that text nodes would always exist by the time a view\`s
creation block had executed. This is not true for text nodes created dynamically
by ICUs because this happens in the update block (in `i18nApply`).
This change ensures such dynamically created nodes are skipped when encountered
too early (as they will be attached later by i18n code anyway).
PR Close#31789
We currently have a handwritten version of the Ivy directive def for NgClass so
we can switch between Ivy and View Engine behavior. This generated code needs to
be kept up-to-date with what the Ivy compiler generates.
PR 30742 recently changed `classMap` such that it now requires allocation of
host binding slots. This means that the `allocHostVars()` function must be
called in the NgClass directive def to match compiler output, but the
handwritten directive def was not updated. This caused a bug where NgClass
was inappropriately overwriting data for other directives because space was
not allocated for its values.
PR Close#31788
This commit bumps the heap size of node/ngc under bazel to 4 GB instead of
the previous 2 GB. This brings it in sync with what the TS compiler uses
in g3, and fixes a bug where ngc would crash with OOM in worker mode.
PR Close#31784
Currently, template expressions and statements have their location
recorded relative to the HTML element they are in, with no handle to
absolute location in a source file except for a line/column location.
However, the line/column location is also not entirely accurate, as it
points an entire semantic expression, and not necessarily the start of
an expression recorded by the expression parser.
To support record of the source code expressions originate from, add a
new `sourceSpan` field to `ASTWithSource` that records the absolute byte
offset of an expression within a source code.
Implement part 2 of [refactoring template parsing for
stability](https://hackmd.io/@X3ECPVy-RCuVfba-pnvIpw/BkDUxaW84/%2FMA1oxh6jRXqSmZBcLfYdyw?type=book).
PR Close#31391
This commit is the final patch of the ivy styling algorithm refactor.
This patch swaps functionality from the old styling mechanism to the
new refactored code by changing the instruction code the compiler
generates and by pointing the runtime instruction code to the new
styling algorithm.
PR Close#30742
Versions of CLI prior to angular/angular-cli@0e339ee did not expose the host.getModifiedResourceFiles() method.
This meant that null was being passed through to the IncrementalState.reconcile() method
to indicate that there were either no changes or the host didn't support that method.
This commit fixes a bug where we were checking for undefined rather than null when
deciding whether any resource files had changed, causing a null reference error to be thrown.
This bug was not caught by the unit testing because the tests set up the changed files
via a slightly different process, not having access to the CompilerHost, and these test
were making the erroneous assumption that undefined indicated that there were no
changed files.
PR Close#31322
Currently developers can use the `By` class to construct common
`DebugElement` query predicates. e.g. `By.directive(MyDirective)`.
The `directive()` and `all()` predicates are currently returning
a predicate that works for `DebugElement` nodes. This return type
is too strict since the predicate is not specific to `DebugElement`
instances and can also apply to `DebugNode` instances.
Meaning that developers are currently able to use the `directive()`
predicate when using `queryAllNodes()`. This is a common practice
but will break when the project is compiled with TypeScript's
`--strictFunctionTypes` flag as the `DebugElement` predicate type
is not assignable to predicates for `DebugNode`. In order to make
these predicates usable with `--strictFuntionTypes` enabled, we
adjust the predicate type to reflect what is actually needed for
evaluation of the predicate.
PR Close#30993
Fixes all TypeScript failures caused by enabling the `--strict`
flag for test source files. We also want to enable the strict
options for tests as the strictness enforcement improves the
overall codehealth, unveiled common issues and additionally it
allows us to enable `strict` in the `tsconfig.json` that is picked
up by IDE's.
PR Close#30993
In View Engine, developers can pass bootstrap and entry components
as nested arrays. e.g.
```ts
export const MyOtherEntryComponents = [A, B, C]
@NgModule({
entryComponents: [MyComp, MyOtherEntryComponents]
})
```
Currently using nested arrays for these properties causes
unexpected errors to be reported in Ivy since the semantic
NgModule checks aren't properly recursing into the nested
entry/bootstrap components. This issue has been unveiled by
enabling the strict function parameter checks.
PR Close#30993
Currently the `ɵɵdefineComponent` method has incorrect type
definitions the `directives` and `pipes` metadata property.
The incorrect types allow developers to pass in already instantiated
`DirectiveDef` or `ComponentDef` objects. This can cause unexpected
failures because the definition internally only expects `Type` objects
and now incorrectly tries to read the `ngDirectiveDef` or `ngComponentDef`
of existing definitions.
This issue has been unveiled by enabling the strict function parameter
types flag, where the directive definitions are determined from each array
element in the `directives` or `pipes` property (which can throw).
PR Close#30993
As part of FW-1265, the `@angular/compiler` package is made compatible
with the TypeScript `--strict` flag. This already unveiled a few bugs,
so the strictness flag seems to help with increasing the overall code health.
Read more about the strict flag [here](https://www.typescriptlang.org/docs/handbook/compiler-options.html)
PR Close#30993
As part of FW-1265, the `@angular/core` package is made compatible
with the TypeScript `--strict` flag. This already unveiled a few bugs,
so the strictness flag seems to help with increasing the overall code health.
Read more about the strict flag [here](https://www.typescriptlang.org/docs/handbook/compiler-options.html)
PR Close#30993
Previously, the usage of `null` and `undefined` keywords in code that is
statically interpreted by ngtsc resulted in a `DynamicValue`, as they were
not recognized as special entities. This commit adds support to interpret
these keywords.
PR Close#31150
The support for decorators that were imported via a namespace,
e.g. `import * as core from `@angular/core` was implemented
piecemeal. This meant that it was easy to miss situations where
a decorator identifier needed to be handled as a namepsaced
import rather than a direct import.
One such issue was that UMD processing of decorators was not
correct: the namespace was being omitted from references to
decorators.
Now the types have been modified to make it clear that a
`Decorator.identifier` could hold a namespaced identifier,
and the corresponding code that uses these types has been
fixed.
Fixes#31394
PR Close#31426
Fixes Ivy matching directives against attribute bindings (e.g. `[attr.some-directive]="foo"`). Works by excluding attribute bindings from the attributes array during compilation. This has the added benefit of generating less code.
**Note:** My initial approach to implementing this was to have a different marker for attribute bindings so that they can be ignored when matching directives, however as I was implementing it I realized that the attributes in that array were only used for directive matching (as far as I could tell). I decided to drop the attribute bindings completely, because it results in less generated code.
PR Close#31541
Data members in TypeScriptServiceHost of Map type should be eagerly
initialized to address issue/24571. This eliminates the need to
constantly check for truthiness and makes code much more readable.
More PRs to follow to address issue/24571.
PR Close#31577
Prior to this commit, default value for LOCALE_ID was not setup for Closure Compiler. In Closure Compiler, we can use `goog.LOCALE` as a default value, which will be replaced at build time with current locale.
PR Close#31519
Currently we reuse the same instruction both for regular property bindings and property bindings on the `host`. The only difference between the two is that when it's on the host we shouldn't support inputs. We have an optional parameter called `nativeOnly` which is used to differentiate the two, however since `nativeOnly` is preceeded by another optional parameter (`sanitizer`), we have to generate two extra parameters for each host property bindings every time (e.g. `property('someProp', 'someValue', null, true)`).
These changes add a new instruction called `hostProperty` which avoids the need for the two parameters by removing `nativeOnly` which is always set and it allows us to omit `sanitizer` when it isn't being used.
These changes also remove the `nativeOnly` parameter from the `updateSyntheticHostBinding` instruction, because it's only generated for host elements which means that we can assume that its value will always be `true`.
PR Close#31550
Add support for indexing elements in the indexing module.
Opening and self-closing HTML tags have their selector indexed, as well
as the attributes on the element and the directives applied to an
element.
PR Close#31240
If zonejs is sent undefined callbacks it proceeds to attempt to call them, then fails, catches it own fail, rewrites the stack to hide the mistake, and reports a TypeError with a callstack unrelated to inputs.
Throw early if the callback is undefined (as can happen if JS or any-ified TS calls zone invokeTask).
Check for undefined onCommplete callback to zonejs jasmine wrapper.
PR Close#31497
Switch back to passing a custom bazel host instead of rewriting one that
is passed to `compile` now that the Angular indexer is stable.
Revert "feat(bazel): allow passing and rewriting an old bazel host"
This reverts commit 0a4c1c8f803a65f5c156af90e67f7d7d68ebc7f8.
PR Close#31496
There are two places in the ngcc processing where it needs to load the
content of a file given by a general path:
* when determining the format of an entry-point.
To do this ngcc uses the value of the relevant property in package.json.
But in the case of `main` it must parse the contents of the entry-point
file to decide whether the format is UMD or CommonJS.
* when parsing the source files for dependencies to determine the order in
which compilation must occur. The relative imports in each file are parsed
and followed recursively, looking for external imports.
Previously, we naively assumed that the path would match the file name exactly.
But actually we must consider the standard module resolution conventions.
E.g. the extension (.js) may be missing, or the path may refer to a directory
containing an index.js file.
This commit fixes both places.
This commit now requires the `DependencyHost` instances to check
the existence of more files than before (at worst all the different possible
post-fixes). This should not create a significant performance reduction for
ngcc. Since the results of the checks will be cached, and similar work is
done inside the TS compiler, so what we lose in doing it here, is saved later
in the processing. The main performance loss would be where there are lots
of files that need to be parsed for dependencies that do not end up being
processed by TS. But compared to the main ngcc processing this dependency
parsing is a small proportion of the work done and so should not impact
much on the overall performance of ngcc.
// FW-1444
PR Close#31509
Prior to this fix, the logic to set the right placeholder format for ICUs was a bit incorrect: if there was a nested ICU in one of the root ICU cases, that led to a problem where placeholders in subsequent branches used the wrong ({$placeholder}) format instead of {PLACEHOLDER} one. This commit updates the logic to make sure we properly transform all placeholders even if nested ICUs are present.
PR Close#31516
Prior to this commit, it was impossible to override providers defined via ModuleWithProviders using TestBed.overrideProvider API. The problem was caused by the fact that we were not taking into account these providers while calculating accumulated set of provider overrides. This commit updates the logic to extract providers from ModuleWithProviders and calculate the final set of overrides taking them into account.
PR Close#31415
Since, 7186f9c01 `compiler-cli` is no longer depending on `shelljs` for
production code. (We still use it in tests and infrastructure/tooling.)
Incidentally, this should also help with #29460.
PR Close#31468
Removes direct calls from one instruction into another, moves the shared logic into a separate function and removes the state getters from the shared function.
This PR resolves FW-1340.
PR Close#31456
Previously, resource paths beginning with '/' (aka "rooted" paths, which
are not actually absolute filesystem paths, but are relative to the
TypeScript project root directory) were not handled correctly. The leading
'/' was stripped and the path was resolved as if it was relative, but with
no containing file for context. This led to resources in different rootDirs
not being found.
Instead, such rooted paths are now resolved without TypeScript's help, by
checking each root directory. A test is added to this effect.
PR Close#31511
When determining if a `main` path points to a UMD or CommonJS
format, the contents of the file need to be loaded and parsed.
Previously, it was assumed that the path referred to the exact filename,
but did not account for normal module resolution semantics, where the
path may be missing an extension or refer to a directory containing an
`index.js` file.
// FW-1444
PR Close#31509
Since `goog.getMsg` does not process ICUs (post-processing is required via goog.i18n.MessageFormat, https://google.github.io/closure-library/api/goog.i18n.MessageFormat.html) and placeholder format used for ICUs and regular messages inside `goog.getMsg` are different, the current implementation (that assumed the same placeholder format) needs to be updated. This commit updates placeholder format used inside ICUs from `{$placeholder}` to `{PLACEHOLDER}` to better align with Closure. ICU placeholders (that were left as is prior to this commit) are now replaced with actual values in post-processing step (inside `i18nPostprocess`).
PR Close#31459
Prior to this commit, the `` unicode symbol that represents `&ngsp` in translations was not handled correctly, i.e. was not replaced with a whitespace, thus appearing on a screen. This commit adds post-processing and replaces the mentioned symbol with a whitespace.
PR Close#31479
This commit adds a test that verifies no translations are generated for bound attributes and also checks (as a part of the `verify` function) that VE and Ivy handle this case the same way.
PR Close#31481
`i18nStart` was calling `allocExpando` even if there was 0 new variable created.
This created a new expando instruction with the value 0 which was later interpreted as the start of a new expando block instead of just skipping 0 instructions.
FW-1417 #resolve
PR Close#31451
Fixes Ivy's return value for `DebugNode.context` not being consistent for the case where there is both a structural directive and a component on a node. In `ViewEngine` the instance of the component would be returned, whereas in Ivy the context of the directive is returned.
Also adds a couple of extra test cases for how `DebugNode.context` deals with directives.
This PR resolves FW-1343.
PR Close#31442
Adds a new `elementContainer` instruction that can be used to avoid two instruction (`elementContainerStart` and `elementContainerEnd`) for `ng-container` that has text-only content. This is particularly useful when we have `ng-container` inside i18n sections.
This PR resolves FW-1105.
PR Close#31444
Paths can be mapped directly to files, which was not being taken
into account when computing `basePaths` for the `EntryPointFinder`s.
Now if a `pathMapping` pattern does not exist or is a file, then we try
the containing folder instead.
Fixes#31424
PR Close#30525
Previously, ngcc had to walk the entire `node_modules` tree looking for
entry-points, even if it only needed to process a single target entry-point
and its dependencies.
This added up to a few seconds to each execution of ngcc, which is noticeable
when being run via the CLI integration.
Now, if an entry-point target is provided, only that target and its entry-points
are considered rather than the whole folder tree.
PR Close#30525
When profiling ngcc it is notable that a large amount of time
is spent dealing with an exception that is thrown (and handled
internally by fs) when checking the existence of a file.
We check file existence a lot in both finding entry-points
and when TS is compiling code. This commit adds a simple
cached `FileSystem`, which wraps a real `FileSystem` delegate.
This will reduce the number of calls through to `fs.exists()` and
`fs.readFile()` on the delegate.
Initial benchmarks indicate that the cache is miss to hit ratio
for `exists()` is about 2:1, which means that we save about 1/3
of the calls to `fs.existsSync()`.
Note that this implements a "non-expiring" cache, so it is not suitable
for a long lived `FileSystem`, where files may be modified externally.
The cache will be updated if a file is changed or moved via
calls to `FileSystem` methods but it will not be aware of changes
to the files system from outside the `FileSystem` service.
For ngcc we must create a new `FileSystem` service
for each run of `mainNgcc` and ensure that all file operations
(including TS compilation) use the `FileSystem` service.
This ensures that it is very unlikely that a file will change
externally during `mainNgcc` processing.
PR Close#30525
This message gets called if a format has already been
compiled and we only want the first. So the message itself
is wrong but it is also not very useful anyway.
PR Close#30525
The ngcc tool adds namespaced imports to files when compiling. The ngtsc
tooling was not processing types correctly when they were imported via
such namespaces. For example:
```
export declare class SomeModule {
static withOptions(...): ModuleWithProviders<ɵngcc1.BaseModule>;
```
In this case the `BaseModule` was being incorrectly attributed to coming
from the current module rather than the imported module, represented by
`ɵngcc1`.
Fixes#31342
PR Close#31367
If a package delcares a class internally on an NgModule, ngcc
needs to be able to add a public export to this class's type.
Previously, if the typing file for the declared is not imported
from the typings entry-point file, then ngcc cannot find it.
Now we try to guess the .d.ts files from the equivalent .js
files.
PR Close#31411
This partially reverts some changes from 71b9371180 (diff-dd469785fca8680a5b33b1e81c5cfd91R1420)
These broke the g3sync of zone.js because we use the output of the TypeScript compiler directly, rather than rely on the rollup commonjs plugin to define the global symbol
PR Close#31453
Non-wild-card path-mappings were not being matched correctly.
Further path-mapped secondary entry-points that
were imported from the associated primary entry-point were not
being martched correctly.
Fixes#31274
PR Close#31450
This is needed in g3 where the translation system is sensitive to the full path of the output.
For Bazel users, we don't want this because it would force them to
disable a Bazel option that prevents using the deprecated ctx.new_file
API
PR Close#31410
Adds a schematic and tslint rule that automatically migrate the consumer from `Renderer` to `Renderer2`. Supports:
* Renaming imports.
* Renaming property and method argument types.
* Casting to `Renderer`.
* Mapping all of the methods from the `Renderer` to `Renderer2`.
Note that some of the `Renderer` methods don't map cleanly between renderers. In these cases the migration adds a helper function at the bottom of the file which ensures that we generate valid code with the same return value as before. E.g. here's what the migration for `createText` looks like.
Before:
```
class SomeComponent {
createAndAddText() {
const node = this._renderer.createText(this._element.nativeElement, 'hello');
node.textContent += ' world';
}
}
```
After:
```
class SomeComponent {
createAndAddText() {
const node = __rendererCreateTextHelper(this._renderer, this._element.nativeElement, 'hello');
node.textContent += ' world';
}
}
function __rendererCreateTextHelper(renderer: any, parent: any, value: any) {
const node = renderer.createText(value);
if (parent) {
renderer.appendChild(parent, node);
}
return node;
}
```
This PR resolves FW-1344.
PR Close#30936
Updates the decision made in #31341; this is for the Angular indexer
inside Google. The indexer currently passes (and ngc-wrapped#compile
accepts) a bazel host to use, but because many methods are overwritten
specially for Angular compilation a better approach is to pass an old
bazel compiler host and siphon methods needed off of it before creating
a new host. This enables that.
PR Close#31381
Judy Bogart
Pete Bacon Darwin
Paul Gschwendtner
crisbeto
Andrew Kushnir
Matias Niemelä
Kara Erickson
Alex Rickabaugh
Pawel Kozlowski
Keen Yee Liau
Ayaz Hafiz
Miško Hevery
Balint Mero
Matt Lewis
Momin Khan
JoostK
nikz99
Jon Wallsten
vikerman
Joey Perrott
George Kalpakas
Hamid
Olivier Combe
Alex Eagle
Michael Prentice