TCB generation occasionally transforms binding expressions twice, which can
result in a `BindingPipe` operation being `resolve()`'d multiple times. When
the pipe does not exist, this caused multiple OOB diagnostics to be recorded
about the missing pipe.
This commit fixes the problem by making the OOB recorder track which pipe
expressions have had diagnostics produced already, and only producing them
once per expression.
PR Close#39517
Previously, the project used for running integration tests for Angular
Elements declared a component that used `ShadowDom` for view
encopsulation, but it did not include any tests to verify that the view
was updated correctly.
This commit adds the missing tests.
PR Close#39452
`ComponentNgElementStrategy` is supposed to call `ngOnChanges()` on the
underlying component instance if available, but not fail if the
component does not have an `ngOnChanges()` method. This works as
expected. However, the test used to verify that was invalid; i.e. the
test would pass even if `ComponentNgElementStrategy` would try to call
`ngOnChanges()` on a component without such a method.
This commit replaces the invalid test with a new one that correctly
verifies that `ComponentNgElementStrategy` does not try to call
`ngOnChanges()`.
PR Close#39452
Previously, the `componentRef` property of `FakeComponentFactory` used
in `elements` tests was initialy set to a spy object with all mock
properties defined as spied methods. Later, the properties where
overwritten to the actual mock values.
This commit simplifies the creation of `componentRef` by correctly using
the arguments of [jasmine.createSpyObj()][1] to specify the desired
shape of the spy object (separating spied properties from methods and
directly providing the mock values).
[1]: https://jasmine.github.io/api/3.5/jasmine.html#.createSpyObj
PR Close#39452
In ViewEngine, SelfSkip would navigate up the tree to get tokens from
the parent node, skipping the child. This restores that functionality in
Ivy. In ViewEngine, if a special token (e.g. ElementRef) was not found
in the NodeInjector tree, the ModuleInjector was also used to lookup
that token. While special tokens like ElementRef make sense only in a
context of a NodeInjector, we preserved ViewEngine logic for now to
avoid breaking changes.
We identified 4 scenarios related to @SkipSelf and special tokens where
ViewEngine behavior was incorrect and is likely due to bugs. In Ivy this
is implemented to provide a more intuitive API. The list of scenarios
can be found below.
1. When Injector is used in combination with @Host and @SkipSelf on the
first Component within a module and the injector is defined in the
module, ViewEngine will get the injector from the module. In Ivy, it
does not do this and throws instead.
2. When retrieving a @ViewContainerRef while @SkipSelf and @Host are
present, in ViewEngine, it throws an exception. In Ivy it returns the
host ViewContainerRef.
3. When retrieving a @ViewContainerRef on an embedded view and @SkipSelf
is present, in ViewEngine, the ref is null. In Ivy it returns the parent
ViewContainerRef.
4. When utilizing viewProviders and providers, a child component that is
nested within a parent component that has @SkipSelf on a viewProvider
value, if that provider is provided by the parent component's
viewProviders and providers, ViewEngine will return that parent's
viewProviders value, which violates how viewProviders' visibility should
work. In Ivy, it retrieves the value from providers, as it should.
These discrepancies all behave as they should in Ivy and are likely bugs
in ViewEngine.
PR Close#39464
There is a compiler transform that downlevels Angular class decorators
to static properties so that metadata is available for JIT compilation.
The transform was supposed to ignore non-Angular decorators but it was
actually completely dropping decorators that did not conform to a very
specific syntactic shape (i.e. the decorator was a simple identifier, or
a namespaced identifier).
This commit ensures that all non-Angular decorators are kepts as-is
even if they are built using a syntax that the Angular compiler does not
understand.
Fixes#39574
PR Close#39577
Tokenized text node may have leading whitespace skipped from their
source-span. But the source-span is used to compute where there are
interpolated blocks, resulting in placeholder nodes whose source-spans
are offset by the amount of skipped characters.
This fix uses the `fullStart` location of text source-spans for computing
the source-span of placeholders, so that they are accurate.
Fixes#39195
PR Close#39486
This commit ensures that when leading whitespace is skipped by
the tokenizer, the original start location (before skipping) is captured
in the `fullStart` property of the token's source-span.
PR Close#39486
The lexer is able to skip leading trivia in the `start` location of tokens.
This makes the source-span more friendly since things like elements
appear to begin at the start of the opening tag, rather than at the
start of any leading whitespace, which could include newlines.
But some tooling requires the full source-span to be available, such
as when tokenizing a text span into an Angular expression.
This commit simply adds the `fullStart` location to the `ParseSourceSpan`
class, and ensures that places where such spans are cloned, this
property flows through too.
PR Close#39486
In an i18n message, two placeholders next to each other must have
an "empty" message-part to separate them. Previously, the source-span
for this message-part was pointing to the wrong original location.
This caused problems in the generated source-maps and lead to extracted
i18n messages from being rendered incorrectly.
PR Close#39486
Prior to this commit, the `cleanUpControl` function (responsible for cleaning up control instance)
was not taking validators into account. As a result, these validators remain registered on a detached
form control instance, thus causing memory leaks. This commit updates the `cleanUpControl` function
logic to also run validators cleanup.
As a part of this change, the logic to setup and cleanup validators was refactored and moved to
separate functions (with completely opposite behavior), so that they can be reused in the future.
This commit doesn't add the `cleanUpControl` calls to all possible places, it just fixes the cases
where this function is being called, but doesn't fully perform a cleanup. The `cleanUpControl`
function calls will be added to other parts of code (to avoid more memory leaks) in a followup PR.
PR Close#39234
When a class with a custom decorator is transpiled to ES5, it looks something like this:
```
var SomeClass = (function() {
function SomeClass() {...};
var SomeClass_1 = __decorate([Decorator()], SomeClass);
SomeClass = SomeClass_1;
return SomeClass;
})();
```
The problem is that if the class also has an Angular decorator that refers to the class itself
(e.g. `{provide: someToken, useClass: SomeClass}`), the generated `setClassMetadata` code will
be emitted after the IIFE, but will still refer to the intermediate `SomeClass_1` variable from
inside the IIFE. This happens, because we generate the `setClassMetadata` call directly from
the source AST which contains identifiers that TS will rename when it emits the ES5 code.
These changes resolve the issue by looking through the metadata AST and cloning any `Identifier`
that is referring to the class. Since TS doesn't have references to the clone, it won't rename
it when transpiling to ES5.
Fixes#39509.
PR Close#39527
When fetch is run in normal mode, the `git-fetch-pack` plumbing command
outputs progress to stderr. Since this is unnecessary progress
information for ng-dev usages, it should be suppressed instead.
PR Close#39503
The fw-testing group should only match on testing directories within the
framework packages, previously it was also matching in other areas of
the codebase.
PR Close#39536
Previously, the documentation for each major Angular version was hosted
on each own Firebase project. This required creating a new project for
each major release and increased the administrative/maintenance cost.
Now that Firebase supports hosting [multiple websites][1] as part of the
same project, we are switching to deploying all major versions to sites
created on `angular-io` project.
This is part of the work needed to prepare angular.io for our
[new versioning/branching process][2] (also tracked in #39366).
[1]: https://firebase.google.com/docs/hosting/multisites
[2]: https://docs.google.com/document/d/197kVillDwx-RZtSVOBtPb4BBIAw0E9RT3q3v6DZkykU
PR Close#39470
This commit breaks up the code in `deploy-to-firebase.js` script, that
we use for deploying angular.io to production, to smaller functions
(instead of a monolithic block). This makes the script easier to
maintain and also makes testing individual operations easier.
The commit also updates the `deploy-to-firebase.spec.js` spec file to
take advantage of the standalone functions to speed up testing by
calling the corresponding function instead of having to spawn a new
process and run the `deploy-to-firebase.js` script with the `--dry-run`
flag.
NOTE: Before updating the tests, I verified that the updated
`deploy-to-firebase.js` script passed the old tests.
PR Close#39470
Different deployment modes (such as `archive` and `next`) are identified
by the different colors used in prominent elements of the page, such as
the topbar and the footer.
Previously, the necessary styles for creating such a deployment mode
"theme" were duplicated for each mode.
This commit simplifies the creation/modification of a deployment mode
theme by introducing a Sass mixin that generates the necessary styles
(when provided with necessary theme colors).
PR Close#39470
Previously, the `deploy-to-firebase.js` script and the accompanying
`deploy-to-firebase.spec.js` spec file were using the `origin` remote
alias in certain commands. This works fine on CI, where `origin` points
to the `angular/angular` GitHub repo, but might not work locally.
This commit ensures that the correct remote is used by explicitly
specifying it by the URL, thus ensuring that the tests will behave
identically on CI and locally.
PR Close#39470
This commit switches the `deploy-to-firebase.sh` script, that we use for
deploying angular.io to production, from Bash to JavaScript. This makes
the script easier to maintain.
For the same reasons, it also switches the `deploy-to-firebase.test.sh`
script, that we use for testing the `deploy-to-firebase` script, from
Bash to JavaScript (using jasmine as the test runner).
Finally, this commit also updates ShellJS to the latest version to get
better error messages (including the actual error) when `exec()` fails.
NOTE: Before switching the test script to JS, I verified that the new
`deploy-to-firebase.js` script passed the tests with the old
`deploy-to-firebase.test.sh` script.
PR Close#39470
When registering an NgModule based on its id, all transitively imported
NgModules are also registered. This commit introduces a visited set to
avoid traversing into NgModules that are reachable from multiple import
paths multiple times.
Fixes#39487
PR Close#39514
The variable declaration for a template context is only needed when it
is referenced from somewhere, so the TCB operation to generate the
declaration is marked as optional.
PR Close#39321
When there is a primary outlet present in the outlets map and the object is also prefixed
with some other commands, the current logic only uses the primary outlet and ignores
the others. This change ensures that all outlets are respected at the
segment level when prefixed with other commands.
PR Close#39456
This commit has a small refactor of some methods in create_url_tree.ts
and adds some test cases, including two that will fail at the moment but
should pass. A follow-up commit will make use of the refactorings to fix
the test with minimal changes.
PR Close#39456
- Improves JSON formatting
- Add reference to font optimization
- Removes `="true"` from boolean command line args.
These are redundant and it can be confusing to why
boolean values need to be provided via a CLI.
PR Close#39427
Currently expressions `$event.foo()` and `this.$event.foo()`, as well as `$any(foo)` and
`this.$any(foo)`, are treated as the same expression by the compiler, because `this` is considered
the same implicit receiver as when the receiver is omitted. This introduces the following issues:
1. Any time something called `$any` is used, it'll be stripped away, leaving only the first parameter.
2. If something called `$event` is used anywhere in a template, it'll be preserved as `$event`,
rather than being rewritten to `ctx.$event`, causing the value to undefined at runtime. This
applies to listener, property and text bindings.
These changes resolve the first issue and part of the second one by preserving anything that
is accessed through `this`, even if it's one of the "special" ones like `$any` or `$event`.
Furthermore, these changes only expose the `$event` global variable inside event listeners,
whereas previously it was available everywhere.
Fixes#30278.
PR Close#39323
This commit updates the docs for the `tView.preOrderHooks` and `tView.preOrderCheckHooks` TView
fields. Current docs are not up-to-date as it was pointed out in #39439.
Closes#39439.
PR Close#39497
This is follow-up from [an earlier discussion](https://github.com/angular/angular/pull/39408#discussion_r511908358).
After some testing, it looks like the type of `Element.attributes` was correct in specifying that it
only has `TextAttribute` instances. This means that the extra checks that filter out `BoundAttribute`
instances from the array isn't necessary. There is another loop a bit further down that actually
extracts the bound i18n attributes.
PR Close#39498
Close#39296
Fix an issue that `markDirty()` will not trigger change detection.
The case is for example we have the following component.
```
export class AppComponent implements OnInit {
constructor(private router: Router) {}
ngOnInit() {
this.router.events
.pipe(filter((e) => e instanceof NavigationEnd))
.subscribe(() => ɵmarkDirty(this));
}
}
export class CounterComponent implements OnInit, OnDestroy {
ngOnInit() {
this.countSubject.pipe(takeUntil(this.destroy)).subscribe((count) => {
this.count = count;
ɵmarkDirty(this);
});
}
```
Then the app navigate from `AppComponent` to `CounterComponent`,
so there are 2 `markDirty()` call at in a row.
The `1st` call is from `AppComponent` when router changed, the
`2nd` call is from `CounterComponent.ngOnInit()`.
And the `markDirty()->scheduleTick()` code look like this
```
function scheduleTick(rootContext, flags) {
const nothingScheduled = rootContext.flags === 0 /* Empty */;
rootContext.flags |= flags;
if (nothingScheduled && rootContext.clean == _CLEAN_PROMISE) {
rootContext.schedule(() => {
...
if (rootContext.flags & RootContextFlags.DetectChanges)
rootContext.flags &= ~RootContextFlags.DetectChanges;
tickContext();
rootContext.clean = _CLEAN_PROMISE;
...
});
```
So in this case, the `1st` markDirty() will
1. set rootContext.flags = 1
2. before `tickContext()`, reset rootContext.flags = 0
3. inside `tickContext()`, it will call `CounterComponent.ngOnint()`,
so the `2nd` markDirty() is called.
4. and the `2nd` scheduleTick is called, `nothingScheduled` is true,
but rootContext.clean is not `_CLEAN_PROMISE` yet, since the `1st` markDirty tick
is still running.
5. So nowhere will reset the `rootContext.flags`.
6. then in the future, any other `markDirty()` call will not trigger the tick, since
`nothingScheduled` is always false.
So `nothingScheduled` means no tick is scheduled, `rootContext.clean === _CLEAN_PROMISE`
means no tick is running.
So we should set the flags to `rootContext` only when `no tick is scheudled or running`.
PR Close#39316
Alex Rickabaugh