There was a subtle bug involving the opt-out class for FormBuilder, which I discovered during the ongoing migration. The types must be structurally the same, because people pass around FormBuilders, in addition to passing around the controls they produce. This PR ensures FormBuilder and UntypedFormBuilder are assignable to each other.
PR Close#45421
In early versions of Angular, it was sometimes necessary to provide a
`moduleId` to `@Component` metadata, and the common pattern for doing this
was to set `moduleId: module.id`. This relied on the bundler to fill in a
value for `module.id`.
However, due to the superficial similarity between `Component.moduleId` and
`NgModule.id`, many users ended up setting `id: module.id` in their
NgModules. This is an anti-pattern that has a few negative effects,
including preventing the NgModule from tree-shaking properly.
This commit changes the compiler to ignore `id: module.id` in NgModules, and
instead provide a warning which suggests removing the line entirely.
PR Close#45024
Previously, the `TraitCompiler` would naively consider a compilation as
failed if either analysis or resolution produced any diagnostics. This
commit adjusts the logic to only consider error diagnostics, which allows
warnings to be produced from `DecoratorHandler`s.
This is a precursor commit to introducing such a warning. As such, the
logic here will be tested in the next commit.
PR Close#45024
Angular contains an NgModule registry, which allows a user to declare
NgModules with string ids and retrieve them via those ids, using the
`getNgModuleById` API.
Previously, we attempted to structure this registration in a clever fashion
to allow for tree-shaking of registered NgModules (that is, those with ids).
This sort of worked due to the accidental alignment of behaviors from the
different tree-shakers involved. However, this trick relies on the
generation of `.ngfactory` files and how they're specifically processed in
various bundling scenarios. We intend to remove `.ngfactory` files, hence
we can no longer rely on them in this way.
The correct solution here is to recognize that `@NgModule({id})` is
inherently declaring a global side-effect, and such classes should not
really be eligible for tree-shaking in the first place. This commit removes
all the old registration machinery, and standardizes on generating a side-
effectful call to `registerNgModuleType` for NgModules that have ids.
There is some risk here that NgModules with unnecessary `id`s may not
tree-shake as a result of this change, whereas they would have in previous
circumstances. The fix here should be to remove the `id` if it's not needed.
Specifying an `id` is a request that the NgModule be retained regardless of
any other references, in case it is later looked up by string id.
PR Close#45024
When `@angular/compiler` processes metadata and compiles a definition field,
it might also choose to return statements that are associated with that
definition, and should be included after the type being compiled. Currently,
the linker ignores these statements, as there are none generated that are
relevant in the linking operation.
A challenge to supporting such associated statements is that the linker
operates on "declare" expressions, and replaces those expressions with other
expressions. It does not have the capability to append statements after the
whole type. The linker actually faces this challenge with statements from
the `ConstantPool` as well, and solves this problem by generating an IIFE
expression that executes the statements and then returns the definition
expression.
Previously, an `EmitScope` processed the definition and converted it to an
expression, as well as collected constant statements from a `ConstantPool`.
A special `IifeEmitScope` implementation was used when emitting into a
context where top-level constant statements couldn't be added at all, and
uses the IIFE strategy in this case.
This commit adds blanket support for associated statements to the linker
using this IIFE strategy. The main `EmitScope` now uses the IIFE strategy to
emit associated statements, and `IifeEmitScope` has been renamed to
`LocalEmitScope`. Now, the `LocalEmitScope` represents constant statements
as associated statements to the main `EmitScope` implementation, so they
get included in the IIFE as well.
Tests are adjusted/added to cover this new behavior. This is a refactoring
commit because no live generated code is affected - there are no cases where
associated statements are present in linked definitions today.
PR Close#45024
The `compileNgModule` operation previously supported a flag `emitInline`,
which controlled whether template scoping information for the NgModule was
emitted directly into the compiled NgModule definition, or whether an
associated statement was generated which patched the information onto the
NgModule definition. Both options are useful in different contexts.
This commit changes this flag to an enum (and renames it), which allows for
a third option - do not emit any template scoping information. This option
is added to better represent the actual behavior of the Angular Linker,
which sometimes configures `compileNgModule` to use the side-effectful
statement generation but which does not actually emit such associated
statements. In other words, the linker effectively does not generate
scoping information for NgModules at all (in some configurations) and this
option more directly expresses that behavior.
This is a refactoring as no generated code is changed as a result of
introducing this flag, due to the linker's behavior of not emitting
associated statements.
PR Close#45024
Previously, the migration only migrated constructor calls. Now, the migration will rewrite every usage, in all contexts. Both ways are technically correct, but migrating all symbols is likely to produce clearer and more readable results.
PR Close#45311
warn developers when they are trying to animate non-animatable CSS
properties so that can more easily understand why something is not being
animated as they would expect it to
resolves#27577
PR Close#45212
The animations package supports adding default parameter values to an animation that will be used as a fallback if some parameters aren't defined. The problem is that they're applied using a spread expression which means that any own property of the animation parameters will override the defaults, even if it resolves to null or undefined. This can lead to obscure errors like "Cannot read property toString of undefined" for an animation that looks like `{params: {foo: undefined}}` with defaults `{foo: 123}`.
I ran into this issue while debugging some test failures on Material.
These changes address the issue by:
1. Applying the defaults if the resolved value is null or undefined.
2. Updating the validation function to use a null check instead of `hasOwnProperty`.
PR Close#45339
* `tree` function now accepts the old root rather than the old
`UrlTree`. The `urlTree` argument was only used to get the `root`.
This change makes it more clear what that pararmeter is used for and
what's actually being used
* Move the `oldRoot` (previously `urlTree`) to be the first argument of `tree`.
This change now mirrors the argument order for `replaceSegment` and
can be read from left to right more easily "in this root,
replace this old segment group with this new segment group".
* Extract `newRoot` to a variable. This just makes it more clear what's
going on at the end rather than combining a bunch of operations into
one.
These changes are being made so that hopefully a future refactor can be
done which does not rely on the `urlTree` argument at all in the
`createUrlTree` function. These refactorings will make it easier to see
1:1 functionlity in these various places.
PR Close#45306
Close#44724
`DebugNode.triggerEventHandler()` should accept the `eventObj` as an
optional parameter. So the user don't have to write code like
```
elem.triggerEventHandler('click', null);
```
PR Close#45279
When authoring Angular templates, developers are likely to be most interested in
the current Directive/Component inputs and outputs, then potential
attributes which would match other directives to the element,
and lastly the plethora of DOM events and attributes.
This change ensures that Angular-specific information appears above DOM
information by prepending the first printable ASCII characters to the
sort text.
Fixes https://github.com/angular/vscode-ng-language-service/issues/1537
PR Close#45293
Consider a file that imports `FormControl` and then never uses it. In that case, we don't need to add the import for `UntypedFormControl`.
By examining constructor calls *first*, we can identify these cases and skip over them.
This will reduce the memory footprint of the migration when run in tsunami, hopefully making OOM errors less likely.
PR Close#45288
Node.js v12 will become EOL on 2022-04-30. As a result, Angular CLI v14 will no longer support Node.js v12.
BREAKING CHANGE:
Support for Node.js v12 has been removed as it will become EOL on 2022-04-30. Please use Node.js v14.15 or later.
PR Close#45286
The `NoNewVersionDetectedEvent` does not imply that the latest version
on the server is the same as the version the client is on (because a
client could be on an older version).
Update the description of the event to better describe what it actually
means (i.e. that the SW didn't find a version it didn't already know
about, but without implying anything about the version the current
client is using).
PR Close#45266
When parsing interpolations, the input string is _decoded_ from what was
in the orginal template. This means that we cannot soley rely on the input
string to compute source spans because it does not necessarily reflect
the exact content of the original template. Specifically, when there is
an HTML entity (i.e. ` `), this will show up in its decoded form
when processing the interpolation (' '). We need to compute offsets
using the original _encoded_ string.
Note that this problem only surfaces in the splitting of interpolations.
The spans to this point have already been tracked accurately. For
example, given the template ` <div></div>`, the source span for the
`div` is already correctly determined to be 6. Only when we encounter
interpolations with many parts do we run into situations where we need
to compute new spans for the individual parts of the interpolation.
PR Close#44811
Currently whenever we insert element we do it directly on the node using `appendChild` or `insertBefore`, however this doesn't work with `<template>` elements where the `template.content` has to be used.
These changes add a few checks to call `appendChild` and `insertBefore` on the `content` of the template.
Fixes#15557.
PR Close#43429
The typed forms migration was previously designed to add `<any>` type parameters to existing forms classes. However, due to some design changes, the new opt-out strategy requires untyped versions of the classes, as introduced in #45205 and #45268.
This PR updates the migration to import these new classes (in an idempotent manner), and replace constructor calls with the new classes. It respects qualified imports as well. Finally, the code has been refactored to move as much common code as possible into `util.ts`.
PR Close#45281
Jasmine logs a warning when there's a `describe` with no tests. These changes fix one such case in the compiler that happens when the tests are run against Windows.
PR Close#45285
The compiler generates code for the Angular runtime in `@angular/core` which
has to be the exact same version, as otherwise there may be version skew
between what the compiler generates and what the runtime supports. This would
result in hard to diagnose problems at runtime. By adding a peer dependency
for `@angular/compiler` on `@angular/core` we can let the package manager
report an error (NPM 7+) or warning (NPM 6, Yarn) during installation to
signal that the set of packages is incompatible.
PR Close#45267
When the service worker checks for an update and finds that the version on the server is the same as
the version locally installed, it currently noops. This change introduces an event which it emits
in this situation which notifies clients a check has occurred without error and no update was found.
PR Close#45216
This commit removes special (undocumented) logic in the Router code that is
meant to prevent duplicate navigations that result from location syncs in
AngularJS/Angular hybrid applications.
The duplicate navigations can occur when both the Router and the AngularJS sync
code detect a location change via a popstate/hashchange event. When this
happens, the Angular Router schedules a navigation to sync itself with
the browser, but the hybrid listener may also schedule an additional
navigation. There are a few reasons this logic should not be included in
the Router:
* This special logic is not tree shakeable so it introduces a bundle
size cost for all applications, most of which don't need it.
* There have been many updates to the routing pipeline to tolerate
duplicate navigations. That is, duplicate navigations can happen and
routing should still complete successfully.
* 0e8548f667
* 9e039ca68b
* The logic is really in the wrong place: The hybrid sync code should be
the location to handle this. If duplicate navigations are meant to be
avoided, the hybrid sync code should have handling to _not_ trigger
duplicate navs.
* This logic _also_ used to exist because the mock location
helper used for test incorrectly triggered popstate events during
router navigations. In order to avoid unexpected behavior in tests, this
logic needed to be added. This incorrect mocking may also have been
put in place because the upgrade module _would_ see a location change
event and trigger a duplicate navigation. The location mock has since been updated to
match real browser behavior so this is no longer necessary. The
upgrade module has also been updated to not trigger duplicate
navigations. The following commits are related to this:
* 202a1a5631
* c6a93001eb
Side note: The `setTimeout` in the location change listener is used to
ensure the ordering of duplicate navigations was consistent. You can see
that the logic being removed here expects the imperative navigation to precede the
popstate/hashchange. With the removal of this code, the `setTimeout` no
longer serves a purpose. However, it has been found that tests can rely
on this behavior (incorrectly) because they expect the navigation to be
complete but in reality, it hasn't even started because the test has not
flushed the timeout. Removing the timeout would be a breaking change as
a result.
PR Close#45240
Adds support for passing in an optional injector when creating an embedded view through `ViewContainerRef.createEmbeddedView` and `TemplateRef.createEmbeddedView`. The injector allows for the DI behavior to be customized within the specific template.
This is a second stab at the changes in #44666. The difference this time is that the new injector acts as a node injector, rather than a module injector.
Fixes#14935.
PR Close#45156
This commit updates the `NgLocalization` token to become tree-shakable (vs using a direct reference to that token in the `providers` section of the `CommonModule`). The `NgLocalization` token is used for apps that use i18n and for other apps it would be excluded from the bundle.
PR Close#45118
model.ts is currently extremely large. This is the first step in an attempt to refactor it to be more easily navigable and reviewable. This commit breaks up `model.ts` into the following new files:
* `model/abstract_model.ts`: The remainder of the model, including the `AbstractControl` base class and helper functions which are used throughout.
* `model/form_control.ts`: `FormControl`, `FormControlOptions`, and helpers, plus the constructor and untyped friends.
* `model/form_array.ts`: `FormArray` and untyped friends.
* `model/form_group.ts`: `FormGroup` and untyped friends.
This first phase is a purely mechanical code move. There is no new code at all, and no interfaces have been separated.
PR Close#45217
The documented command for updating the forms gold files was outdated
and didn't work. Since this command list can easily become outdated,
remove the individual commands in favor of the simpler global scripts.
PR Close#45198
For legacy browsers, we still use the eventNames array to patch event instead of
using `Object.getOwnPropertyNames()` to keep backward compatibility.
PR Close#40962
Zone.js supports the google closure compiler in the advance optimization mode,
to prevent closure compiler modify the `onProperty` name such as `Element.prototype.onclick`,
Zone.js implements the `onProperty` patch logic by declaring all the
event names in the source code, this increases the bundle size and also require
updating the event names array to keep the information updated.
Now google closure compiler has the required event names defined in the built-in
externs files, so zone.js can use more simple implementation and decrease the bundle size
of zone.js (about 4k).
PR Close#40962
The router used to wait for the resolvers to complete and take the last
value. The changes here take only the first
emitted value of every resolver and proceed the navigation. This matches
how other guards work in the `Router` code.
Resolves https://github.com/angular/angular/issues/44643
BREAKING CHANGE: Previously, resolvers were waiting to be completed
before proceeding with the navigation and the Router would take the last
value emitted from the resolver.
The router now takes only the first emitted value by the resolvers
and then proceeds with navigation. This is now consistent with `Observables`
returned by other guards: only the first value is used.
PR Close#44573
Currently, there is a freestanding `getRawValue` function which examines the type of the control. This is an issue for refactoring `model.ts` because it creates unnecessary dependencies between the `AbstractControl` classes. It is cleaner to simply add this method to the model hierarchy and call it directly, and will ease upcoming refactoring.
PR Close#45200
