The language service reports an error when a directive's template
context is missing a member that is being used in a template (e.g. if
`$implicit` is being used with a template context typed as `any`).
While this diagnostic message is valuable, typing template contexts
loosely as `any` or `object` is very widespread in community packages,
and often still compiles correctly, so reporting the diagnostic as an
error may be misleading to users.
This commit changes the diagnostic to be a warning, and adds additional
information about how the user can eliminate the warning entirely -- by
refining the template context type.
Closes https://github.com/angular/vscode-ng-language-service/issues/572
PR Close#35036
Sometimes, a request for definitions will return multiple of the same
definition. This can happen in at least the cases of
- two-way bindings (one of the same definition for the property and
event binding)
- multiple template binding expressions in the same attribute
- something like "*ngFor="let i of items; trackBy: test" has two
template bindings, resulting in two template binding ASTs at the
same location (the attribute span). The language service then parses
both of these bindings individually, resulting in two independent
but identical definitions. For more context, see https://github.com/angular/angular/pull/34847#discussion_r371006680.
This commit prunes duplicate definitions by signing definitions with
their location, and checking if that location signature has been seen in
a previous definition returned to the client.
PR Close#34995
In #35004, we started ignoring yarn's engines check for `yarn install`
in AIO's `test-production.sh` script to fix a failure in the
`aio_monitoring_stable` CI job. (See #35004 for details.)
It turns out that the version of yarn used on the stable branch (1.17.3)
`--ignore-engines` is needed on all yarn commands (including `yarn
run`). Thus, #35004 is not enough to fix the failures.
New example failure: https://circleci.com/gh/angular/angular/604341
This commit turns of the engines check for the whole
`aio_monitoring_stable` CI job to fix the failure and make the job more
robust.
PR Close#35033
In #34974 the top level dependency on `@babel/core` was bumped to
7.8.3. This commit ensures that the package.json that gets included
in the `@angular/localize` distributable is at the same version.
PR Close#35008
Previously, when the benchmark tests ran outside of Bazel, developers
had the posibility to control how the tests run through command line
options. e.g. `--dryrun`. This no longer works reliable in Bazel where
command line arguments are not passed to the text executable.
To make the global options still usable (as they could be still useful
in some cases), we just pass them through the Bazel `--test_env`. This
reduces the code we need to read the command line, but still preserves
the flexibility in a Bazel idiomatic way.
PR Close#34753
Currently, based on the file names it's not quite clear whether
a given `.spec.ts` file runs benchmark perf or benchmark e2e
functionality tests. To disambiguate these, we use new file
suffixs. i.e. `e2e-spec.ts` and `perf-spec.ts`.
PR Close#34753
Currently we run all benchmark perf tests in CircleCI. Since we do not
collect any results, we unnecessarily waste CI/RBE resources. Instead,
we should just not run benchmark perf tests in CI, but still run the
functionality e2e tests which ensure that benchmarks are not broken.
We can do this by splitting the perf and e2e tests into separate
files/targets.
PR Close#34753
There are different `DebugNode`/`DebugElement` implementations (and
associated helper functions) for ViewEngine and Ivy. Additionally, these
classes/functions, which are defined inside the `core` package, are
imported by the `platform-browser` package.
Previously, this code was not tree-shaken as expected in Ivy. #30130
partially addressed the issue, but only for the case where `core` and
`platform-browser` end up in the same closure after webpack's scope
hoisting. In cases where this is not the case, our webpack/terser based
tooling is not capable of tree-shaking it.
This commit fixes the problem, by ensuring that the code retained in Ivy
mode (due to the cross-package import) does not unnecessarily reference
`DebugNode`/`DebugElement`, allowing the code to be tree-shaken away.
This results in a 7.6KB reduction in the size of the main angular.io
bundle.
Jira issue: [FW-1802](https://angular-team.atlassian.net/browse/FW-1802)
PR Close#35003
the commit causes lint on upstream/master to fail. We need to fix the script to not check too many past commits, but that
will have to wait until a follow up PR.
PR Close#35017
In #34955, we switched to Node.js v12 on master and 9.0.x. This causes
the `aio_monitoring_job` CI job (which checks out files from the stable
branch; currently 8.2.x) to start failing yarn's engines check (since
the 8.2.x branch expects Node.js version <11).
Example failure: https://circleci.com/gh/angular/angular/602499
Since the job is expected to run with files from both the stable and the
master branches (and since the version of Node.js is not important
here), this commit uses the `--ignore-engines` option to prevent
failures due to Node.js version mismatch.
NOTE:
Typically, the stable and master branch are on the same Node.js version,
because related PRs land on both master and the patch branch. One
exception is during RC periods, when the stable branch is different than
the patch branch. These periods are usually short, but in the case of
9.0.0 the period has lasted several months causing the CI environments
between master and the stable branch to get significantly out-of-sync.
PR Close#35004
* Added a /tools/saucelabs/sauce-service.sh script that manages the sauce-connect as a service which is used by the karma-saucelabs.js wrapper to start the service.
* Added /tools/saucelabs/README.md that covers the details of SauceLabs karma testing with Bazel.
PR Close#34769
earlier p sigly was used to define a paragraph element which was not clear what we want to refer changed it to paragrapg element for better understanding
Fixes#34922
PR Close#34927
This condition is being removed as it currently is failing to work as
expected with pullapprove at the moment. The plan/expectation is that
after a bit of troubleshooting, we will be able to add in the condition
once more. Removing the condition actually returns us to the same
state we were in with CODEOWNERS in which draft PRs are also assigned
reviews.
PR Close#35012
We are migrating to PullApprove for our PR review management in an attempt
to allow for more granular and equitable code review assignments across the
team. Currently this migration is equivalent in the review assignments
it will create. Once stable, our expectation is that we will be able to
take advantage of PullApproves additional features for things like staged
reviews.
PR Close#34814
The message now gives concrete advice to developers who
experience the error due to running multiple simultaneous builds
via webpack.
Fixes#35000
PR Close#35001
This commit adds support for completions of properties on `$event`
variables in bound outputs.
This is the second major PR to support completions for `$event`
variables (https://github.com/angular/vscode-ng-language-service/issues/531).
The final completion support that must be provided is for `$event`
variables in bindings targeting DOM events, like `(click)`.
PR Close#34570
This commit makes `findOutputBinding` a utility function for the
language service, which will be used by the `expression_diagnostics`
module in #34570. Keeping the function in `locate_symbol` results in a
circular dependency between `expression_diagnostics` and
`locate_symbol`.
PR Close#34997
We had some logic for generating and passing in the `elIndex` parameter into the `hostBindings` function, but it wasn't actually being used for anything. The only place left that had a reference to it was the `StylingBuilder` and it only stored it without referencing it again.
PR Close#34969
Component's decorator handler exposes `preanalyze` method to preload async resources (templates, stylesheets). The logic in preanalysis phase may throw `FatalDiagnosticError` errors that contain useful information regarding the origin of the problem. However these errors from preanalysis phase were not intercepted in TraitCompiler, resulting in just error message text be displayed. This commit updates the logic to handle FatalDiagnosticError and transform it before throwing, so that the result diagnostic errors contain the necessary info.
PR Close#34801
For the structural directive, the 'path' will contain multiple `BoundDirectivePropertyAst` which depends on the number of directive property in the attribute value(e.g. '*ngFor="let item of []; trackBy: test;"', it has 2 `BoundDirectivePropertyAst`, 'ngForOf' and 'ngForTrackBy').
PR Close#34847
This update increases the main bundle by ~0.6KB
payload size snapshot:
456581 Jan 24 22:07 dist/main-es2015.38c39f92eab2fcc8c835.js
541321 Jan 24 22:06 dist/main-es5.38c39f92eab2fcc8c835.js
52487 Jan 24 22:05 dist/polyfills-es2015.b374ef3555a700a97add.js
146193 Jan 24 22:05 dist/polyfills-es5.c7dc569e6c646e42fade.js
2987 Jan 24 22:05 dist/runtime-es2015.29be4028399ae41ba25e.js
2981 Jan 24 22:05 dist/runtime-es5.29be4028399ae41ba25e.js
PR Close#34966
the test failures most likely result from the babel updates in the previous commit.
it does look like we lost the file path from the error message, which is something that we
should follow up no in a separate change.
PR Close#34974
Previously we relied on this package being hoisted and available, which is error prone and it would
be just a matter of time before the build would break due rehoisting of deps upon future npm package.json
changes.
PR Close#34974
It's not clear how this ever worked (npm hoisting is a suspect though), but this dep is required because
one of the tests imports @babel/generator in the ts sources.
The lack of this dep is breaking builds on the master branch.
More discussion about this issue on Slack: https://angular-team.slack.com/archives/C07DT5M6V/p1579934766007500
PR Close#34974
As part of the effort to tighten the API surface of
`TypeScriptServiceHost` in preparation for the migration to Ivy, I realized
some recently added APIs are not strictly needed.
They can be safely removed without sacrificing functionality.
This allows us to clean up the code, especially in the implementation of
QuickInfo, where the `TypeScriptServiceHost` is leaked outside of the
`LanguageService` class.
This refactoring also cleans up some duplicate code where the QuickInfo
object is generated. The logic is now consolidated into a simple
`createQuickInfo` method shared across two different implementations.
PR Close#34941
Previously we would write to class/style as strings `element.className` and `element.style.cssText`. Turns out that approach is good for initial render but not good for updates. Updates using this approach are problematic because we have to check to see if there was an out of bound write to style and than perform reconciliation. This also requires the browser to bring up CSS parser which is expensive.
Another problem with old approach is that we had to queue the DOM writes and flush them twice. Once on element advance instruction and once in `hostBindings`. The double flushing is expensive but it also means that a directive can observe that styles are not yet written (they are written after directive executes.)
The new approach uses `element.classList.add/remove` and `element.style.setProperty/removeProperty` API for updates only (it continues to use `element.className` and `element.style.cssText` for initial render as it is cheaper.) The other change is that the styling changes are applied immediately (no queueing). This means that it is the instruction which computes priority. In some circumstances it may result in intermediate writes which are than overwritten with new value. (This should be rare)
Overall this change deletes most of the previous code and replaces it with new simplified implement. The simplification results in code savings.
PR Close#34804
This change introduces several functions for manipulating items in an array in an efficient (binary search) way.
- `arraySplice` a faster version of `Array.splice()`.
- `arrayInsert` a faster version of `Array.splice(index, 0, value)`.
- `arrayInsert2` a faster version of `Array.splice(index, 0, value1, value2)`.
- `arrayInsertSorted` a way to insert a value into sorted list.
- `arrayRemoveSorted` a way to remove a value from a sorted list.
- `arrayIndexOfSorted` a way to find a value in a sorted list.
- `ArrayMap` Efficient implementation of `Map` as an `Array`.
- `arrayMapSet`, `arrayMapGet`, `arrayMapIndexOf`, and `arrayMapDelete` for manipulating `ArrayMap`s.
PR Close#34804
NOTE: This change must be reverted with previous deletes so that it code remains in build-able state.
This change deletes old styling code and replaces it with a simplified styling algorithm.
The mental model for the new algorithm is:
- Create a linked list of styling bindings in the order of priority. All styling bindings ere executed in compiled order and than a linked list of bindings is created in priority order.
- Flush the style bindings at the end of `advance()` instruction. This implies that there are two flush events. One at the end of template `advance` instruction in the template. Second one at the end of `hostBindings` `advance` instruction when processing host bindings (if any).
- Each binding instructions effectively updates the string to represent the string at that location. Because most of the bindings are additive, this is a cheap strategy in most cases. In rare cases the strategy requires removing tokens from the styling up to this point. (We expect that to be rare case)S Because, the bindings are presorted in the order of priority, it is safe to resume the processing of the concatenated string from the last change binding.
PR Close#34616
Ayaz Hafiz
ayazhafiz
Judy Bogart
frosty
George Kalpakas
Pete Bacon Darwin
Paul Gschwendtner
Igor Minar
Greg Magolan
ajitsinghkaler
Joey Perrott
Kristiyan Kostadinov
Andrew Kushnir
Alan Agius
Turtuvshin Byambaa
ivanwonder
philonor
Keen Yee Liau
Miško Hevery