Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a new pull request by comparing changes across two branches #234

Merged
merged 36 commits into from
Aug 28, 2024

Conversation

GulajavaMinistudio
Copy link

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

bjarkler and others added 30 commits August 26, 2024 09:04
…57454)

Angular applications that are AngularJS hybrids are currently unable to
adopt Trusted Types due to violations eminating from an innerHTML
assignment in the @angular/upgrade package. This commit allows
developers of such applications to optionally ignore this class of
violations by configuring the Trusted Types header to allow the new
angular#unsafe-upgrade policy.

Note that the policy is explicitly labeled as unsafe as it does not in
any way mitigate the security risk of using AngularJS in an Angular
application, but does unblock Trusted Types adoption enabling XSS
protection for other parts of the application.

The implementation follows the approach taken in @angular/core;
see packages/core/src/util/security.

PR Close #57454
Previously, the component handler was processing and resolving stylesheets
referenced via `styleUrl`/`styleUrls` multiple times when generating the
compiler metadata for components. The style resource information collection
for such styles has been further consolidated to avoid repeat resource loader
resolve calls which potentially could be expensive. Further optimization is
possible for the inline style case. However, inline styles here only require
AST traversal and no potentially expensive external resolve calls.

PR Close #57502
Updates the repo to the release candidate of TypeScript 5.6.

PR Close #57507
Updated Angular CLI help contents.

PR Close #57508
…from spamming console (#55697)

When a browser extension is updated it becomes invalidated on currently open pages. If that extension then tries to send a message to those pages through `chrome.runtime.sendMessage(..)` then an error is thrown in the console

For Angular DevTools, this results in spamming the console with "Uncaught Error: Extension context invalidated." errors.

This commit catches that error and removes the event listener that triggers the `chrome.runtime.sendMessage(...)` call.

PR Close #55697
#57062)

add an url query param to perform a search on api references directly via the url

PR Close #57062
…57416)

This commit updates the implementations of `autoDetectChanges` to be
shared between the zone-based and zoneless fixtures. This now allows
`autoDetect` to be turned off for zoneless fixtures after it was
previously on because the host view is no longer directly attached to
`ApplicationRef`.

PR Close #57416
This commit moves the ngZone onError subscription to the base fixture
implementation. While this subscription isn't necessary for zoneless, it does
no harm because the observable never emits.

PR Close #57416
This commit removes the abstract base class and two separate
implementations of `ComponentFixture` for zone vs zoneless. Now that the
behaviors have gotten close enough to the same, the diverged concrete
implementations serve less value. Instead, the different behaviors can
be easily handled in if/else branches. The difference is now limited to
the default for `autoDetect` and how `detectChanges` functions.

PR Close #57416
Disabling `checkNoChanges` in `ComponentFixture.detectChanges` was an
error for the zoneless fixture since it was not yet working. This now
allows checkNoChanges to be disabled. This option isn't really
used/shouldn't be used by anyone except the FW so marked as a refactor.

PR Close #57416
…eless (#57416)

When disabling autodetect (not recommeneded) with zoneless,
`fixture.detectChanges` would previously not refresh the fixture's component.

PR Close #57416
…56507)

This configures whether or not to preserve whitespace content when extracting messages from Angular templates in the legacy (View Engine) extraction pipeline.

This includes several bug fixes which unfortunately cannot be landed without changing message IDs in a breaking fashion and are necessary to properly trim whitespace. Instead these bug fixes are included only when the new flag is disabled.

PR Close #56507
…Visitor` (#56507)

When disabling `i18nPreserveSignificantWhitespaceForLegacyExtraction` I was looking at a test case with ICU messages containing leading and trailing whitespace:

```angular
<div i18n>
  {apples, plural, =other {I have many apples.}}
</div>
```

This would historically generate two messages:

```javascript
const MSG_TMP = goog.getMsg('{apples, plural, =other {I have many apples.}}');
const MSG_FOO = goog.getMsg(' {$ICU} ', { 'ICU': MSG_TMP });
```

But I found that I was getting just one message:

```javascript
const MSG_TMP = goog.getMsg(' {apples, plural, =other {I have many apples.}} ');
```

This is arguably an improvement, but changed the messages and message IDs, which isn't desirable with this option. I eventually traced this back to the `isIcu` initialization in [`i18n_parser.ts`](/packages/compiler/src/i18n/i18n_parser.ts):

```typescript
const context: I18nMessageVisitorContext = {
  isIcu: nodes.length == 1 && nodes[0] instanceof html.Expansion,
  // ...
};
```

[`_I18nVisitor.prototype.visitExpansion`](/packages/compiler/src/i18n/i18n_parser.ts) uses this to decide whether or not to generate a sub-message for a given ICU expansion:

```typescript
if (context.isIcu || context.icuDepth > 0) {
  // Returns an ICU node when:
  // - the message (vs a part of the message) is an ICU message, or
  // - the ICU message is nested.
  const expPh = context.placeholderRegistry.getUniquePlaceholder(`VAR_${icu.type}`);
  i18nIcu.expressionPlaceholder = expPh;
  context.placeholderToContent[expPh] = {
    text: icu.switchValue,
    sourceSpan: icu.switchValueSourceSpan,
  };
  return context.visitNodeFn(icu, i18nIcu);
}

// Else returns a placeholder
// ICU placeholders should not be replaced with their original content but with the their
// translations.
// TODO(vicb): add a html.Node -> i18n.Message cache to avoid having to re-create the msg
const phName = context.placeholderRegistry.getPlaceholderName('ICU', icu.sourceSpan.toString());
context.placeholderToMessage[phName] = this.toI18nMessage([icu], '', '', '', undefined);
const node = new i18n.IcuPlaceholder(i18nIcu, phName, icu.sourceSpan);
return context.visitNodeFn(icu, node);
```

Note that `isIcu` is the key condition between these two cases and depends on whether or not the ICU expansion has any siblings. The introduction of `WhitespaceVisitor` to `I18nMetaVisitor` trims insignificant whitespace, including empty text nodes not adjacent to an ICU expansion (from [`WhitespaceVisitor.prototype.visitText`](/packages/compiler/src/ml_parser/html_whitespaces.ts)):

```typescript
const isNotBlank = text.value.match(NO_WS_REGEXP);
const hasExpansionSibling =
  context && (context.prev instanceof html.Expansion || context.next instanceof html.Expansion);

if (isNotBlank || hasExpansionSibling) {
  // Transform node by trimming it...
  return trimmedNode;
}

return null; // Drop node which is empty and has no ICU expansion sibling.
```

`hasExpansionSibling` was intended to retain empty text nodes leading or trailing an ICU expansion, however `context` was `undefined`, so this check failed and the leading / trailing text nodes were dropped. This resulted in trimming the ICU text by dropping the leading / trailing whitespace nodes. Having only a single ICU expansion with no leading / trailing text nodes caused `_I18nVisitor` to initialize `isIcu` incorrectly and caused it to generate one message instead of two.

`WhitespaceVisitor` is supposed to get this context from `visitAllWithSiblings`. So the fix here is to make sure `WhitespaceVisitor` is always visited via this function which provides the required context. I updated all usage sites to make sure this context is use consistently and implemented the `WhitespaceVisitor.prototype.visit` method to throw when the context is missing to make sure we don't encounter a similar mistake in the future.

Unfortunately this broke one compliance test. Specifically the [`icu_logic/icu_only.js`](/home/douglasparker/Source/ng/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/icu_only.js) test which changed from generating:

```javascript
function MyComponent_Template(rf, ctx) {
  if (rf & 1) {
    i0.ɵɵi18n(0, 0);
  }
  // ...
}
```

To now generating:

```javascript
function MyComponent_Template(rf, ctx) {
  if (rf & 1) {
    i0.ɵɵtext(0, " ");
    i0.ɵɵi18n(1, 0);
    i0.ɵɵtext(2, "\n");
  }
  // ...
}
```

This test uses the default value `preserveWhitespaces: false` (`i18nPreserveSignificantWhitespaceForLegacyExtraction` should not affect compiled JS output, we already retain significant whitespace there). So what this indicates to me is that ICU logic is already broken because it's not preserving significant whitespace in this case. My change is probably a bug fix, but one which would affect the compiled runtime, which is not in scope here. The root cause is because using `visitAllWithSiblings` everywhere means the context is retained correctly in this case and the whitespace is leading/trailing an ICU message, therefore it is retained per the logic of `WhitespaceVisitor.prototype.visitText` I mentioned eariler.

To address this, I left one usage of `WhitespaceVisitor` using `html.visitAll` instead of `visitAllWithSiblings` to retain this bug. I has to lossen the assertion I put in `WhitespaceVisitor.prototype.visit` to make this possible, but it should still throw by default when misused, which is the important part.

PR Close #56507
Prepend 'auto' to value of the `sizes` prop generated by NgOptimizedImage, when image has a responsive srcset and is lazy-loaded.

PR Close #57479
…s. (#57480)

BREAKING CHANGE: Render default fallback with empty `projectableNodes`.

When passing an empty array to `projectableNodes` in the `createComponent` API, the default fallback content of the `ng-content` will be rendered if present. To prevent rendering the default content, pass `document.createTextNode('')` as a `projectableNode`.

For example:

```ts
// The first ng-content will render the default fallback content if present
createComponent(MyComponent. { projectableNodes: [[], [secondNode]] });

// To prevent projecting the default fallback content:
createComponent(MyComponent. { projectableNodes: [[document.createTextNode('')], [secondNode]] });

```

Fixes #57471

PR Close #57480
…57493)

In some cases apps need to schedule a bit later the loading of the animation module. This private token will allow to investigate which other strategy could be useful.

PR Close #57493
See associated pull request for more information.

PR Close #57514
…der tree detection (#57518)

Previously, if an application had DOM Nodes injected into it from other frames, DevTools would fail to parse component trees with the render tree strategy properly because of an instanceof Node check that the framework performs.

Now we check for instanceof Node before even calling framework debug APIs on DOM nodes so that we can skip nodes that come from other frames entirely.

PR Close #57518
…l queries (#57525)

Adds initial migration logic to convert decorator query declarations to
signal queries.

We will re-use more part of the signal input migration in follow-ups, to
properly migrate, and e.g. even handle control flow

PR Close #57525
Properly handles queries with multiple results, by extracting the
type from the `QueryList`.

Also adds more tests and handles imports.

PR Close #57525
…#57530)

This commit makes a small update to the route matching algorithm to
avoid running the matcher function of a route twice.

fixes #57511

PR Close #57530
…57526)

The `fw-testing` group should not trigger for schematic changes.

PR Close #57526
…g logic (#57537)

This commit updates a directive mock instance to include an extra field that a compiler code was expecting, which caused issues while processing elements with local refs and exported directives.

PR Close #57537
See associated pull request for more information.

PR Close #57542
See associated pull request for more information.

PR Close #57543
This commit reduces the bundle size of `@angular/core` by 19.5 MB by excluding sourcemaps from the schematics.

Since the `esbuild` rule doesn't allow disabling sourcemaps directly, we work around this by setting the sourcemaps to `external`. Afterward, we filter the output to include only the `.js` files.

PR Close #57545
…57546)

When we create the LView for a component, we track it in the `TRACKED_LVIEWS` map. It gets untracked when it is destroy, but if it throws during creation, the user won't have access to a `ComponentRef` in order to clean it up.

These changes automatically untrack the related LViews if the component couldn't be created.

PR Close #57546
We had some tests that were leaking LViews, because we were testing things like `createComponent`, but not destroying them afterwards. These changes clean up most of them, although there are a handful still left that I didn't have time to fully track down.

PR Close #57546
josephperrott and others added 6 commits August 27, 2024 13:31
Update to the latest version of @angular/build-tooling

PR Close #57551
Since the change to the after render hooks, the "this" context must be
correct for the destroy to work.

fixes #57552

PR Close #57554
Instead of using the reference that existing when `FetchBackend` is setup.

fixes #57527

PR Close #57531
…to signal queries (#57525)" (#57555)

This reverts commit 6f5b435.

Reason: breaks g3

PR Close #57555
)

This change updates the timers used in the coalescing and hybrid mode
schedulers to run in the zone above Angular rather than the root zone.
Running the timers in the root zone makes them impossible to flush when
using `fakeAsync` and also may make them invisible to other zones in the
hierarchy that might have desirable behaviors such as task/perf tracking.

fixes #56767

BREAKING CHANGE: The timers that are used for zone coalescing and hybrid
mode scheduling (which schedules an application state synchronization
when changes happen outside the Angular zone) will now run in the zone
above Angular rather than the root zone. This will mostly affect tests
which use `fakeAsync`: these timers will now be visible to `fakeAsync`
and can be affected by `tick` or `flush`.

PR Close #57553
@GulajavaMinistudio GulajavaMinistudio merged commit 02c2db2 into angular-indonesia:main Aug 28, 2024
4 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.