Re-enables the dynamic queries migration, now that we have all of the necessary framework changes in place.
Also moves the logic that identifies static queries out of the compiler and into the static queries migration, because that's the only place left that's using it.
PR Close#32992
Currently the undecorated-classes-with-di migration leverages NGC in order
to work with metadata resolution. Since NGC by default tries to resolve referenced
resources on initialization of the underlying TS program, it can result in unexpected
migration failures due to missing resource files.
This is especially an issue since the CLI wraps the `AngularCompilerProgram` with
special logic (i.e. to support SCSS preprocessing etc.). We don't have all of this since
we instantiate a vanilla NGC program.
The solution to the problem is to simply treat resource requests as valid, and returning
a fake content. The migration is not dependent on templates or stylesheets.. so it's the
simplest and most robust solution.
Fixes#32826
PR Close#32953
ec4381d explicitly set `enableIvy: false` for all migrations inside
the core package. This actually hides migration issues because the
migration itself should ensure that it instantiates the right
compiler program if it relies on `@angular/compiler-cli`.
We should remove these options from all migration tests to
ensure that we catch issues with migrations running in version
9 where Ivy is enabled by default.
e.g. e5636a322c
was accidentally hidden due to the `enableIvy: false` option.
PR Close#32954
ec4381d enabled Ivy by default. This is problematic as migrations
like `static-queries` depend on the `AngularCompilerProgram` (NGC)
in order to perform the migration from version 7 to version 8.
In order to ensure that the migration always runs with NGC
(and doesn't get the `NgtscProgram`), we need to explicitly disable
ivy when creating the `@angular/compiler-cli` program for the migration.
This code is still relevant even though the update from version 7
to version 8 landed. Developers can run `ng update` from version 7
and immediately get to version 9 where Ivy is enabled by default (and in
that case we need to ensure that ngtsc is not accidentally used).
Similar to
e5636a322c.
PR Close#32954
Initially the `missing-injectable` migration was only being used
in google3. Wiring the migration up in the CLI migrations was
planned to be done in a follow-up.
PR Close#32349
Currently the undecorated classes migration decorates base classes if no
explicit constructor is defined on all classes in the inheritance chain.
We only want to decorate base classes which define a constructor that is
inherited. Additionally for best practice, all classes in between the class
that inherits the constructor and the one that defines it are also decorated.
PR Close#32319
The `undecorated-classes-with-di` migration currently creates invalid object literals from parsed
NGC metadata files if there are object literal properties with keys that contain special characters.
e.g. consider a decorated base class with a host binding using `[class.X]`. Currently the migration
parses and converts the metadata to TypeScript code but incorrectly uses `[class.X]` unquoted as
identifier.
PR Close#32319
Apparently the names of the bazel test targets in the schematics are
incorrect. This commit updates the target names to match their bazel
package name.
PR Close#32318
ec4381dd40 enabled Ivy by default. This is
problematic as migrations like `undecorated-classes-with-di` depend on the
`AngularCompilerProgram` (NGC) in order to perform the migration from
version 8 to version 9. In order to ensure that the migration always runs
with NGC (and doesn't get the `NgtscProgram`), we need to explicitly disable
ivy when creating the `@angular/compiler-cli` program for the migration.
PR Close#32318
This commit switches the default value of the enableIvy flag to true.
Applications that run ngc will now by default receive an Ivy build!
This does not affect the way Bazel builds in the Angular repo work, since
those are still switched based on the value of the --define=compile flag.
Additionally, projects using @angular/bazel still use View Engine builds
by default.
Since most of the Angular repo tests are still written against View Engine
(particularly because we still publish VE packages to NPM), this switch
also requires lots of `enableIvy: false` flags in tsconfigs throughout the
repo.
Congrats to the team for reaching this milestone!
PR Close#32219
Initially the plan was to have a migration that adds `@Injectable()` to
all pipes in a CLI project so that the pipes can be injected in Ivy
similarly to how it worked in view engine.
Due to the planned refactorings which ensure that `@Directive`, `@Component`
and `@Pipe` also have a factory definition, this migration is no longer
needed for Ivy. Additionally since it is already disabled (due to
572b54967c) and we have a more generic
migration (known as `missing-injectable)` that could do the same as
`injectable-pipe`, we remove the migration from the code-base.
PR Close#32184
Introduces a new migration schematic that follows the given
migration plan: https://hackmd.io/@alx/S1XKqMZeS.
First case: The schematic detects decorated directives which
inherit a constructor. The migration ensures that all base
classes until the class with the explicit constructor are
properly decorated with "@Directive()" or "@Component". In
case one of these classes is not decorated, the schematic
adds the abstract "@Directive()" decorator automatically.
Second case: The schematic detects undecorated declarations
and copies the inherited "@Directive()", "@Component" or
"@Pipe" decorator to the undecorated derived class. This
involves non-trivial import rewriting, identifier aliasing
and AOT metadata serializing
(as decorators are not always part of source files)
PR Close#31650
Moves the `renderer_to_renderer2` migration google3 tslint rule
into the new `google3` directory. This is done for consistency
as we recently moved all google3 migration rules into a new
`google3` folder (see: f69e4e6f77).
PR Close#31817
Creates a separate bazel target for the google3 migration
tests. The benefit is that it's faster to run tests for
public migrations in development. Google3 lint rules are
usually another story/implementation and the tests are quite
slow due to how TSLint applies replacements.
Additionally if something changes in the google3 tslint rules,
the tests which aren't affected re-run unnecessarily.
PR Close#31817
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
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
Currently the `RecursiveAstVisitor` that is part of the template expression
parser does not _always_ properly pass through the context that can be
specified when visting a given expression. Only a handful of AST types
pass through the context while others are accidentally left out. This causes
unexpected and inconsistent behavior and basically makes the `context`
parameter not usable if the type of template expression is not known.
e.g. the template variable assignment migration currently depends on
the `RecursiveAstVisitor` but sometimes breaks if developers use
things like conditionals in their template variable assignments.
Fixes#31043
PR Close#31085
The "async" around a couple tests was removed because of the merge conflict in one of the previous commits (80394ce08b). This change restores missing "async"s.
PR Close#30762
fix(@schematics/angular): TypeScript related migrations should cater for BOM
In the CLI `UpdateRecorder` methods such as `insertLeft`, `remove` etc.. accepts positions which are not offset by a BOM. This is because when a file has a BOM a different recorder will be used https://github.com/angular/angular-cli/blob/master/packages/angular_devkit/schematics/src/tree/recorder.ts#L72 which caters for an addition offset/delta.
The main reason for this is that when a developer is writing a schematic they shouldn't need to compute the offset based if a file has a BOM or not and is handled out of the box.
Example
```ts
recorder.insertLeft(5, 'true');
```
However this is unfortunate in the case if a ts SourceFile is used and one uses `getWidth` and `getStart` method they will already be offset by 1, which at the end it results in a double offset and hence the problem.
Fixes#30713
PR Close#30719
Currently if a project has source-files with syntax failures and the migration
has been started on a broken project, we silently migrate *as much as possible*,
but never notify the developer that the project was in a broken state and that
he can re-run the migration after the failures are fixed.
Additionally the template strategy does not need to exit gracefully if it detects
Angular semantic diagnostics on generated files (template type checking). These
diagnostics are not relevant for the query timing analysis.
PR Close#30628
We are removing the prompt for the `static-query` migration and make the
template strategy the migration strategy for the migration. The usage
strategy is good for best-practices, but for now we want to ensure that
the migration is a seamless as possible and that is only achievable my
re-using the same logic that View Engine uses for determining the
timing of a query.
PR Close#30628
Currently we try to parse CLI workspace configurations gracefully by
using the native `JSON.parse()` method. This means that the CLI workspace
configuration needs to follow the strict JSON specification because otherwise
the migrations would not be able to find TypeScript configurations in the CLI
project where JSON5 workspace configurations are supported.
In order to handle such workspace configurations, we leverage the JSON
parsing logicfrom the `@angular-devkit/core` which is also used by the CLI.
PR Close#30582
Slightly improves the messages for the static-query migration in order
to make the terminal output less verbose but more helpful. Unfortunately
we are limited in what we can print due to the devkit not providing much
utilities for printing good messages from a migration schematic.
PR Close#30458
Currently if something fails in the selected strategy (e.g. AOT failures),
the migration currently accidentally falls back to the test strategy. This
is not helpful as we want to give developers the possibility to re-run
the migration after fixing potential AOT failures.
PR Close#30458
Preserve compatibility with rollup_bundle rule.
Add missing npm dependencies, which are now enforced by the strict_deps plugin in tsc_wrapped
PR Close#30370
Currently we always just set the timing to `false` if we aren't
able to analyze a given call expression or new expression. e.g.
```ts
ngOnInit() {
thirdPartyCallSync(() => this.query.doSomething())
}
```
In that case the `thirdPartyCallSync` function comes from the `node_modules`
and is only defined through types while there is no code for the
actual function logic that can be analyzed. This makes it impossible
to tell whether the given call expression actually causes the specified
arrow function to be executed synchronously or not. In order to be able
to make this better, we now peek into the passed arrow function and
check for a synchronous query usage. If so, we set the query timing to
static and mark it as ambiguous. This ensures that the usage strategy is
less "magical" and more correct with third-party code.
Additionally since functions like `setTimeout` are not analyzable but known
to be asynchronous, there is a hard-coded list of known functions which
shouldn't be marked as ambiguous.
Resolves FW-1214. As planned within https://hackmd.io/hPiLWpPlQ4uynC1luIBdfQ
PR Close#30215
Currently the static-query migration ignores queries declared on getters
or setters as these are not part of a `PropertyDeclaration`. We need to
handle these queries in order to cover all queries within a given project.
The usage strategy is not able to detect timing for queries on accessors,
so we add a TODO and print a message. The template strategy is able
to detect the proper timing for such queries because it's not dependent
on detecting the usage of the query.
Resolves FW-1215
PR Close#30327
The static-query template strategy leverages the AOT compiler
in order to determine the query timing. Unfortunately the AOT
compiler has open bugs that can cause unexpected failures which
make the template strategy unusable in rare cases. These rare
exceptions need to be handled gracefully in order to avoid confusion
and to provide a more smooth migration.
Additionally migration strategy setup failures are now reported with
stack traces as the `ng update` command does not print stack traces.
This makes it easier to reproduce and report migration issues.
PR Close#30269
Currently when someone has a call expression within the `ngOnInit` call
and we try to peek into that function with respect to the current function
context, the schematic errors because a call expression argument is
undefined. This is valid because the target function declaration defines
that parameter with a default value. In order to fix this, we need to
respect parameter default values.
PR Close#30269
572b54967c changed how the schematic
tests are executed. Tests no longer use the schematic collection
that is also used by the CLI `ng update` command and therefore
the migration collection could technically be invalid.
In order to ensure that the public migration collection is guaranteed
to work and to avoid duplication within two schematic collections, the
changes are partially reverted and only the disabled `injectable-pipe`
schematic has its own collection.
PR Close#30198
Currently we always prompt when the static-query migration runs. This is not
always needed because some applications do not even use `ViewChild` or
`ContentChild` queries and it just causes confusion if developers need to
decide on a migration strategy while there is nothing to migrate.
In order to avoid this confusion, we no longer prompt for a strategy
if there are no queries declared within the project.
PR Close#30254
Currently the `static-query` migrations fails at the final step of
updating a query when the query already specifies options which
cannot be transformed easily. e.g. the options are computed through
a function call: `@ViewChild(..., getQueryOpts());` or `@ViewChild(..., myOptionsVar)`.
In these cases we technically could add additionally logic to update
the query options, but given that this is an edge-case and it's
potentially over-engineering the migration schematic, we just
always add a TODO for the timing and print out the determined
query timing in the console. The developer in that case just needs
to manually update the logic for the query options to contain the
printed query timing.
Potentially related to: https://github.com/angular/angular-cli/issues/14298
PR Close#30178
Disables the injectable pipe migration until we can decide whether this is the right solution for Ivy. Rolling it out properly will involve a more detailed plan and more changes like updating the styleguide, scaffolding schematics etc.
Context for the new `test-migrations.json`: since we use the `migrations.json` both for the real migrations and for tests, it doesn't allow us to disable a schematic, but continue running its tests. This change adds the test-specific file so that we can continue running the `injectable-pipe` tests, even though the schematic itself is disabled.
PR Close#30180
Currently the injectable pipe schematic generates invalid imports like `import import { Pipe, PipeTransform, Injectable } from '@angular/core'; from '@angular/core';`. The issue wasn't caught by the unit tests, because the invalid import still contains the valid one.
Fixes#30159.
PR Close#30170
Currently when someone runs `ng update` with the static-query migration,
the migration can fail with an error saying that the `AOT` compiler could not
be created. This can happen if the CLI project contains a test `tsconfig.json`
that is picked up by the schematic.
Due to the fact that spec tsconfig files cannot be ran with NGC (e.g. test
components are not part of a module; not all source files are guaranteed to
be included), test `tsconfig` projects will now use a new `test` migration
strategy where all queries within tests are left untouched and a TODO is added.
PR Close#30034
Currently the `template-var-assignment` migration incorrectly warns if
the template writes to a property in the component that has the same
`ast.PropertyWrite´ name as a template input variable but different
receiver. e.g.
```html
<!-- "someProp.element" will be incorrectly reported as template variable assignment -->
<button *ngFor="let element of list" (click)="someProp.element = null">Reset</button>
```
Similarly if an output writes to a component property with the same name as a
template input variable, but the expression is within a different template scope,
the schematic currently incorrectly warns. e.g.
```html
<button *ngFor="let element of list">{{element}}</button>
<!-- The "element = null" expression does not refer to the "element" template input variable -->
<button (click)="element = null"></button>
```
PR Close#30026
Currently the `template-strategy` for the static query migration uses the
Angular compiler in order to determine the query timing. This is problematic
as the AngularCompilerProgram also collects metadata for referenced
component stylesheets which aren't necessarily present. e.g. in a CLI
project the component can reference a Sass file. It's not guaranteed
that the standalone Angular compiler plugin supports Sass without
custom logic that is brought in by the Angular CLI webpack plugin.
In order to avoid any failures for invalid stylesheets, we just disable
normalizing of all referenced stylesheets.
PR Close#29876
Currently there are two available migration strategies for the `static-query`
schematic. Both have benefits and negatives which depend on what the
developer prefers. Since we can't decide which migration strategy is the
best for a given project, the developer should be able to select a specific
strategy through a simple choice prompt.
In order to be able to use prompts in a migration schematic, we need to
take advantage of the "inquirer" package which is also used by the CLI
schematic prompts (schematic prompts are usually only statically defined
in the schema). Additionally the schematic needs to be made "async"
because with prompts the schematic can no longer execute synchronously
without implementing some logic that blocks the execution.
PR Close#29876
Currently if there are multiple source files within a given
TypeScript source file, only the last template in the source
file is checked as we store templates in a `Map` with the
source file paths as keys.
This is problematic as multiple templates can live within the
same source file and we therefore accidentally overwrite
existing entries in the resolved templates map.
PR Close#29841
Introduces a new strategy for the `static-query` schematic that
is enabled by default. In order to provide a migration that works
for the most Angular applications and makes the upgrade as easy
as possible, the template strategy leverages the view engine
Angular compiler logic in order to determine the query timing
that is currently used within applications using view engine.
PR Close#29815
Queries can technically be also accessed within component templates
e.g.
```html
<my-comp [binding]="myQuery"></my-comp>
```
In that case the query with the property "myQuery" is accessed
statically and needs to be marked with `static: true`. There are
other edge cases that need to be handled as the template property
read doesn't necessarily resolve to the actual query property.
For example:
```html
<foo #myQuery></foo>
<my-comp [binding]="myQuery"></my-comp>
```
In this scenario the binding doesn't refer to the actual query
because the template reference variable takes precedence. The
query doesn't need to be marked with "static: true" this time.
This commit ensures that the `static-query` migration schematic
now handles this cases properly. Also template property reads
that access queries from within a `<ng-template>` are ignored
as these can't access the query before the view has been initialized.
Resolves FW-1216
PR Close#29713
Kristiyan Kostadinov
Paul Gschwendtner
Alex Rickabaugh
Andrew Kushnir
Alan Agius
Alex Eagle
Adam Plumer
Greg Magolan