The `gulp format*` tasks have been broken since 5eb742621. These include
the `gulp format:enforce` task, which is what runs on CI to enforce
consistent code style. Here is what (I believe) happened:
- I assume formatting was failing in 5eb742621 (moving `zone.js` into
`angular/angular`). The reason must have been that
[this glob pattern][1] matches `packages/zone.js/` (which is a
directory) and passes it to `clang-format` claiming it is a file.
- I further assume that in an attempt to fix the issue,
`gulp-clang-format` was updated to the latest version (1.0.27) in
5eb742621.
- `gulp format:enforce` stopped complaining, so everyone thought
formatting was fine and moved on.
- Formatting still wasn't fine, but the task completed successfully
nevertheless 😱
- The reason is that angular/gulp-clang-format@55b697c5c (and subsequent
commits) changed the way the `done()` callback was called, leaving it
to `clang-format` to call it (while previously it was also called when
the associated stream ended).
- In the old version of `clang-format` that we are using (1.0.41), there
is a bug (which has been fixed in angular/clang-format@4cce2c4ee):
The callback is not called
[unless the process exits with an error][2].
One can also see that the `gulp format:enforce` task is not completed in
`gulp lint`. Example output from [build 374722][3]:
```
yarn gulp lint
...
Starting 'format:enforce'...
Starting 'validate-commit-messages'...
...
Finished 'validate-commit-messages' after 833 ms
Starting 'tools:build'...
Finished 'tools:build' after 1.75 s
Starting 'tslint'...
Finished 'tslint' after 19 s
Done in 21.82s.
```
Notice that all tasks have a corresponding "Finished X` log, except for
`format:enforce`.
For reference:
The problem was originally reported by @ocombe on Slack ([discussion][4]).
---
This commit fixes the issue by downgrading `gulp-clang-format` to
1.0.23. The linting failures due to formatting issues will be addressed
in subsequent commits.
[1]: https://github.com/angular/angular/blob/a8f3b317f/tools/gulp-tasks/format.js#L13
[2]: https://github.com/angular/clang-format/blob/b8c7df0b7/index.js#L95
[3]: https://circleci.com/gh/angular/angular/374722
[4]: https://angular-team.slack.com/archives/C042EU9T5/p1561480241191000
PR Close#31295
Adds `zone.js` as valid scope for commit messages. This
is necessary because the `zone.js` repository has been
moved into the mono-repo and future changes should be
categorized properly through commit messages.
Currently the pre-commit git hook or CircleCI will fail when
`zone.js` is used as commit scope.
PR Close#31277
If an entry-point has missing dependencies then it cannot be
processed and is marked as invalid. Similarly, if an entry-point
has dependencies that have been marked as invalid then that
entry-point too is invalid. In all these cases, ngcc should quietly
ignore these entry-points and continue processing what it can.
Previously, if an entry-point had more than one entry-point that
was transitively invalid then ngcc was crashing rather than
ignoring the entry-point.
PR Close#31276
The Angular runtime frequently calls into user code (for example, when
writing to a property binding). Since user code can throw errors, calls to
it are frequently wrapped in a try-finally block. In Ivy, the following
pattern is common:
```typescript
enterView();
try {
callUserCode();
} finally {
leaveView();
}
```
This has a significant problem, however: `leaveView` has a side effect: it
calls any pending lifecycle hooks that might've been scheduled during the
current round of change detection. Generally it's a bad idea to run
lifecycle hooks after the application has crashed. The application is in an
inconsistent state - directives may not be instantiated fully, queries may
not be resolved, bindings may not have been applied, etc. Invariants that
the app code relies upon may not hold. Further crashes or broken behavior
are likely.
Frequently, lifecycle hooks are used to make assertions about these
invariants. When these assertions fail, they will throw and "swallow" the
original error, making debugging of the problem much more difficult.
This commit modifies `leaveView` to understand whether the application is
currently crashing, via a parameter `safeToRunHooks`. This parameter is set
by modifying the above pattern:
```typescript
enterView();
let safeToRunHooks = false;
try {
callUserCode();
safeToRunHooks = true;
} finally {
leaveView(..., safeToRunHooks);
}
```
If `callUserCode` crashes, then `safeToRunHooks` will never be set to `true`
and `leaveView` won't call any further user code. The original error will
then propagate back up the stack and be reported correctly. A test is added
to verify this behavior.
PR Close#31244
Our module resolution prefers `.js` files over `.d.ts` files because
occasionally libraries publish their typings in the same directory
structure as the compiled JS files, i.e. adjacent to each other.
The standard TS module resolution would pick up the typings
file and add that to the `ts.Program` and so they would be
ignored by our analyzers. But we need those JS files, if they
are part of the current package.
But this meant that we also bring in JS files from external
imports from outside the package, which is not desired.
This was happening for the `@fire/storage` enty-point
that was importing the `firebase/storage` path.
In this commit we solve this problem, for the case of imports
coming from a completely different package, by saying that any
file that is outside the package root directory must be an external
import and so we do not analyze those files.
This does not solve the potential problem of imports between
secondary entry-points within a package but so far that does
not appear to be a problem.
PR Close#30591
Rather than passing a number of individual arguments, we can
just pass an `EntryPointBundle`, which already contains them.
This is also a precursor to using more of the properties in the bundle.
PR Close#30591
This will allow users of the `EntryPointBundle` to use some of the `EntryPoint`
properties without us having to pass them around one by one.
PR Close#30591
Previously we expected the constructor parameter `decorators`
property to be an array wrapped in a function. Now we also support
an array not wrapped in a function.
PR Close#30591
Some packages do not actually provide a `typings` field in their
package.json. But TypeScript naturally infers the typings file from
the location of the JavaScript source file.
This commit modifies ngcc to do a similar inference when finding
entry-points to process.
Fixes#28603 (FW-1299)
PR Close#30591
There are scenarios where it is not possible for ngcc to guess the format
or configuration of an entry-point just from the files on disk.
Such scenarios include:
1) Unwanted entry-points: A spurious package.json makes ngcc think
there is an entry-point when there should not be one.
2) Deep-import entry-points: some packages allow deep-imports but do not
provide package.json files to indicate to ngcc that the imported path is
actually an entry-point to be processed.
3) Invalid/missing package.json properties: For example, an entry-point
that does not provide a valid property to a required format.
The configuration is provided by one or more `ngcc.config.js` files:
* If placed at the root of the project, this file can provide configuration
for named packages (and their entry-points) that have been npm installed
into the project.
* If published as part of a package, the file can provide configuration
for entry-points of the package.
The configured of a package at the project level will override any
configuration provided by the package itself.
PR Close#30591
Previously each test relied on large shared mock file-systems, which
makes it difficult to reason about what is actually being tested.
This commit breaks up these big mock file-systems into smaller more
focused chunks.
PR Close#30591
To improve cross platform support, all file access (and path manipulation)
is now done through a well known interface (`FileSystem`).
For testing a number of `MockFileSystem` implementations are provided.
These provide an in-memory file-system which emulates operating systems
like OS/X, Unix and Windows.
The current file system is always available via the static method,
`FileSystem.getFileSystem()`. This is also used by a number of static
methods on `AbsoluteFsPath` and `PathSegment`, to avoid having to pass
`FileSystem` objects around all the time. The result of this is that one
must be careful to ensure that the file-system has been initialized before
using any of these static methods. To prevent this happening accidentally
the current file system always starts out as an instance of `InvalidFileSystem`,
which will throw an error if any of its methods are called.
You can set the current file-system by calling `FileSystem.setFileSystem()`.
During testing you can call the helper function `initMockFileSystem(os)`
which takes a string name of the OS to emulate, and will also monkey-patch
aspects of the TypeScript library to ensure that TS is also using the
current file-system.
Finally there is the `NgtscCompilerHost` to be used for any TypeScript
compilation, which uses a given file-system.
All tests that interact with the file-system should be tested against each
of the mock file-systems. A series of helpers have been provided to support
such tests:
* `runInEachFileSystem()` - wrap your tests in this helper to run all the
wrapped tests in each of the mock file-systems.
* `addTestFilesToFileSystem()` - use this to add files and their contents
to the mock file system for testing.
* `loadTestFilesFromDisk()` - use this to load a mirror image of files on
disk into the in-memory mock file-system.
* `loadFakeCore()` - use this to load a fake version of `@angular/core`
into the mock file-system.
All ngcc and ngtsc source and tests now use this virtual file-system setup.
PR Close#30921
Enables remote caching for CI jobs.
This configuration:
always reads from build cache on CI
only write to build cache for local builds for non-PR CI run
PR Close#31204
In #31118, some vendor-specific selectors were combined in one rule-set.
As pointed out in [this comment][1], this would result in the whole
rule-set being ignored by all browsers, since one invalid/unrecognized
selector invalidates the declaration block.
This commit fixes it by defining a separate rule-set per selector. The
list of vendor-specific selectors is also adjusted to better target the
currently supported browsers.
[1]: https://github.com/angular/angular/pull/31118/files#r296923652
PR Close#31252
No longer uses docker in order to pull down the toolchain configs
for remote build execution. We don't need to make docker a prerequisite
for working on the Angular repository since we can leverage the checked-in
toolchain configurations from the `@bazel-toolchains` repository.
PR Close#31251
Prior to this commit, the logic to extract query information from class fields used an instance of regular Error class to throw an error. As a result, some useful information (like reference to a specific field) was missing. Replacing Error class with FatalDiagnosticError one makes the error more verbose that should simplify debugging.
PR Close#31123
after update to rules_nodejs 0.32.1, @types are no longer automatically discovered by ngc-wrapped (which uses parts of ts_library) so this test needed updating so that the types files it generates is added as an explicit dep
PR Close#31019
This option is changed to true in Bazel 0.27 and exposes a possible
regression in Bazel 0.27.0.
Error observed is in npm_package target `//packages/common/locales:package`:
```
ERROR: /home/circleci/ng/packages/common/locales/BUILD.bazel:13:1: Assembling
npm package packages/common/locales/package failed: No usable spawn strategy found
for spawn with mnemonic SkylarkAction. Your --spawn_strategyor --strategy flags
are probably too strict. Visit https://github.com/bazelbuild/bazel/issues/7480 for
migration advises
```
Suspect is https://github.com/bazelbuild/rules_nodejs/blob/master/internal/npm_package/npm_package.bzl#L75-L82:
```
execution_requirements = {
# Never schedule this action remotely because it's not computationally expensive.
# It just copies files into a directory; it's not worth copying inputs and outputs to a remote worker.
# Also don't run it in a sandbox, because it resolves an absolute path to the bazel-out directory
# allowing the .pack and .publish runnables to work with no symlink_prefix
# See https://github.com/bazelbuild/rules_nodejs/issues/187
"local": "1",
},
```
PR Close#31019
...tsetse now falsely asserting on some lines in a few tests such as packages/core/schematics/test/injectable_pipe_migration_spec.ts.
```
await runMigration();
expect(tree.readContent('/index.ts'))
```
it asserts that "await is required on promise" on the 2nd line when there is no promise there
PR Close#31019
Brings in ts_library fixes required to get angular/angular building after 0.32.0:
typescript: exclude typescript lib declarations in node_module_library transitive_declarations
typescript: remove override of @bazel/tsetse (+1 squashed commit)
@npm//node_modules/foobar:foobar.js labels changed to @npm//:node_modules/foobar/foobar.js with fix for bazelbuild/rules_nodejs#802
also updates to rules_rass commit compatible with rules_nodejs 0.32.0
PR Close#31019
George Kalpakas
Paul Gschwendtner
Pete Bacon Darwin
Alex Rickabaugh
Pawel Kozlowski
Misko Hevery
Romain Freydiger
Umair Sarfraz
Schlemmer
Olivier Combe
Kapunahele Wong
Joey Perrott
Andrew Kushnir
Santosh Yadav
Greg Magolan